From: Lennart Poettering Date: Sat, 30 Nov 2013 18:45:32 +0000 (+0100) Subject: bus: support temporarily const errors that don't need to be freed but require deep... X-Git-Tag: v209~1185 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=79f8d3d2ce51e992493f2d354a5764262c9d564a;hp=b57bdedc87c763aba1b5e8dc5396bfa3ac7d5086 bus: support temporarily const errors that don't need to be freed but require deep copies This should fix issues with incorrectly copying bus error messages out of sd_bus_message objects. Original bug found by: Djalal Harouni --- diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c index 968e80df7..45dff8088 100644 --- a/src/libsystemd-bus/bus-error.c +++ b/src/libsystemd-bus/bus-error.c @@ -210,20 +210,20 @@ bool bus_error_is_dirty(sd_bus_error *e) { if (!e) return false; - return e->name || e->message || e->need_free; + return e->name || e->message || e->_need_free != 0; } _public_ void sd_bus_error_free(sd_bus_error *e) { if (!e) return; - if (e->need_free) { + if (e->_need_free > 0) { free((void*) e->name); free((void*) e->message); } e->name = e->message = NULL; - e->need_free = false; + e->_need_free = 0; } _public_ int sd_bus_error_set(sd_bus_error *e, const char *name, const char *message) { @@ -244,7 +244,7 @@ _public_ int sd_bus_error_set(sd_bus_error *e, const char *name, const char *mes if (message) e->message = strdup(message); - e->need_free = true; + e->_need_free = 1; finish: return -bus_error_name_to_errno(name); @@ -268,7 +268,7 @@ int bus_error_setfv(sd_bus_error *e, const char *name, const char *format, va_li if (format) vasprintf((char**) &e->message, format, ap); - e->need_free = true; + e->_need_free = 1; finish: return -bus_error_name_to_errno(name); @@ -299,7 +299,13 @@ _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) { assert_return(!bus_error_is_dirty(dest), -EINVAL); - if (!e->need_free) + /* + * _need_free < 0 indicates that the error is temporarily const, needs deep copying + * _need_free == 0 indicates that the error is perpetually const, needs no deep copying + * _need_free > 0 indicates that the error is fully dynamic, needs deep copying + */ + + if (e->_need_free == 0) *dest = *e; else { dest->name = strdup(e->name); @@ -311,7 +317,7 @@ _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) { if (e->message) dest->message = strdup(e->message); - dest->need_free = true; + dest->_need_free = 1; } finish: @@ -380,7 +386,7 @@ static void bus_error_strerror(sd_bus_error *e, int error) { } if (x == m) { - if (e->need_free) { + if (e->_need_free > 0) { /* Error is already dynamic, let's just update the message */ free((char*) e->message); e->message = x; @@ -395,14 +401,14 @@ static void bus_error_strerror(sd_bus_error *e, int error) { return; } - e->need_free = true; + e->_need_free = 1; e->name = t; e->message = x; } } else { free(m); - if (e->need_free) { + if (e->_need_free > 0) { char *t; /* Error is dynamic, let's hence make the message also dynamic */ @@ -444,7 +450,7 @@ _public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) { k = errno_to_bus_error_name_new(error, (char**) &e->name); if (k > 0) - e->need_free = true; + e->_need_free = 1; else if (k < 0) { *e = BUS_ERROR_OOM; return -error; @@ -480,7 +486,7 @@ int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_lis k = errno_to_bus_error_name_new(error, (char**) &e->name); if (k > 0) - e->need_free = true; + e->_need_free = 1; else if (k < 0) { *e = BUS_ERROR_OOM; return -ENOMEM; @@ -496,12 +502,12 @@ int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_lis r = vasprintf(&m, format, ap); if (r >= 0) { - if (!e->need_free) { + if (e->_need_free <= 0) { char *t; t = strdup(e->name); if (t) { - e->need_free = true; + e->_need_free = 1; e->name = t; e->message = m; return -error; diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c index 639b9a621..a1e6c9f97 100644 --- a/src/libsystemd-bus/bus-message.c +++ b/src/libsystemd-bus/bus-message.c @@ -609,6 +609,8 @@ _public_ int sd_bus_message_new_method_error( goto fail; } + t->error._need_free = -1; + *m = t; return 0; @@ -709,6 +711,8 @@ int bus_message_new_synthetic_error( goto fail; } + t->error._need_free = -1; + *m = t; return 0; @@ -3804,6 +3808,9 @@ int bus_message_parse_fields(sd_bus_message *m) { return -EBADMSG; r = message_peek_field_string(m, error_name_is_valid, &ri, &m->error.name); + if (r >= 0) + m->error._need_free = -1; + break; case SD_BUS_MESSAGE_HEADER_DESTINATION: diff --git a/src/libsystemd-bus/test-bus-error.c b/src/libsystemd-bus/test-bus-error.c index 9c0f4e015..16c75a3f6 100644 --- a/src/libsystemd-bus/test-bus-error.c +++ b/src/libsystemd-bus/test-bus-error.c @@ -26,6 +26,12 @@ int main(int argc, char *argv[]) { _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL, second = SD_BUS_ERROR_NULL; + const sd_bus_error const_error = SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_EXISTS, "const error"); + const sd_bus_error temporarily_const_error = { + .name = SD_BUS_ERROR_ACCESS_DENIED, + .message = "oh! no", + ._need_free = -1 + }; assert_se(!sd_bus_error_is_set(&error)); assert_se(sd_bus_error_set(&error, SD_BUS_ERROR_NOT_SUPPORTED, "xxx") == -ENOTSUP); @@ -45,7 +51,10 @@ int main(int argc, char *argv[]) { assert_se(sd_bus_error_is_set(&error)); assert_se(!sd_bus_error_is_set(&second)); + assert_se(second._need_free == 0); + assert_se(error._need_free > 0); assert_se(sd_bus_error_copy(&second, &error) == -ENOENT); + assert_se(second._need_free > 0); assert_se(streq(error.name, second.name)); assert_se(streq(error.message, second.message)); assert_se(sd_bus_error_get_errno(&second) == ENOENT); @@ -53,6 +62,28 @@ int main(int argc, char *argv[]) { assert_se(sd_bus_error_is_set(&second)); sd_bus_error_free(&error); + sd_bus_error_free(&second); + + assert_se(!sd_bus_error_is_set(&second)); + assert_se(const_error._need_free == 0); + assert_se(sd_bus_error_copy(&second, &const_error) == -EEXIST); + assert_se(second._need_free == 0); + assert_se(streq(const_error.name, second.name)); + assert_se(streq(const_error.message, second.message)); + assert_se(sd_bus_error_get_errno(&second) == EEXIST); + assert_se(sd_bus_error_has_name(&second, SD_BUS_ERROR_FILE_EXISTS)); + assert_se(sd_bus_error_is_set(&second)); + sd_bus_error_free(&second); + + assert_se(!sd_bus_error_is_set(&second)); + assert_se(temporarily_const_error._need_free < 0); + assert_se(sd_bus_error_copy(&second, &temporarily_const_error) == -EACCES); + assert_se(second._need_free > 0); + assert_se(streq(temporarily_const_error.name, second.name)); + assert_se(streq(temporarily_const_error.message, second.message)); + assert_se(sd_bus_error_get_errno(&second) == EACCES); + assert_se(sd_bus_error_has_name(&second, SD_BUS_ERROR_ACCESS_DENIED)); + assert_se(sd_bus_error_is_set(&second)); assert_se(!sd_bus_error_is_set(&error)); assert_se(sd_bus_error_set_const(&error, "Posix.Error.EUCLEAN", "Hallo") == -EUCLEAN); diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h index 648b955cf..1cce9c59b 100644 --- a/src/systemd/sd-bus.h +++ b/src/systemd/sd-bus.h @@ -42,7 +42,7 @@ typedef struct sd_bus_creds sd_bus_creds; typedef struct { const char *name; const char *message; - int need_free; + int _need_free; } sd_bus_error; /* Flags */