diff --git a/src/basic/macro.h b/src/basic/macro.h index 68d8b062e87..6e3966ff486 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -396,8 +396,12 @@ static inline int __coverity_check_and_return__(int condition) { if (!p) \ return NULL; \ \ - assert(p->n_ref > 0); \ - p->n_ref++; \ + /* For type check. */ \ + unsigned *q = &p->n_ref; \ + assert(*q > 0); \ + assert(*q < UINT_MAX); \ + \ + (*q)++; \ return p; \ } diff --git a/src/libsystemd/sd-bus/bus-track.c b/src/libsystemd/sd-bus/bus-track.c index bc36673b839..4c44162108c 100644 --- a/src/libsystemd/sd-bus/bus-track.c +++ b/src/libsystemd/sd-bus/bus-track.c @@ -40,7 +40,6 @@ struct sd_bus_track { "arg0='", name, "'") static struct track_item* track_item_free(struct track_item *i) { - if (!i) return NULL; @@ -49,7 +48,8 @@ static struct track_item* track_item_free(struct track_item *i) { return mfree(i); } -DEFINE_TRIVIAL_CLEANUP_FUNC(struct track_item*, track_item_free); +DEFINE_PRIVATE_TRIVIAL_REF_UNREF_FUNC(struct track_item, track_item, track_item_free); +DEFINE_TRIVIAL_CLEANUP_FUNC(struct track_item*, track_item_unref); DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(track_item_hash_ops, char, string_hash_func, string_compare_func, struct track_item, track_item_free); @@ -165,13 +165,13 @@ DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_bus_track, sd_bus_track, track_free); static int on_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus_error *error) { sd_bus_track *track = userdata; - const char *name, *old, *new; + const char *name; int r; assert(message); assert(track); - r = sd_bus_message_read(message, "sss", &name, &old, &new); + r = sd_bus_message_read(message, "sss", &name, NULL, NULL); if (r < 0) return 0; @@ -180,7 +180,7 @@ static int on_name_owner_changed(sd_bus_message *message, void *userdata, sd_bus } _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { - _cleanup_(track_item_freep) struct track_item *n = NULL; + _cleanup_(track_item_unrefp) struct track_item *n = NULL; struct track_item *i; const char *match; int r; @@ -190,14 +190,8 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { i = hashmap_get(track->names, name); if (i) { - if (track->recursive) { - unsigned k = track->n_ref + 1; - - if (k < track->n_ref) /* Check for overflow */ - return -EOVERFLOW; - - track->n_ref = k; - } + if (track->recursive) + track_item_ref(i); bus_track_remove_from_queue(track); return 0; @@ -207,9 +201,14 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { if (r < 0) return r; - n = new0(struct track_item, 1); + n = new(struct track_item, 1); if (!n) return -ENOMEM; + + *n = (struct track_item) { + .n_ref = 1, + }; + n->name = strdup(name); if (!n->name) return -ENOMEM; @@ -241,8 +240,7 @@ _public_ int sd_bus_track_add_name(sd_bus_track *track, const char *name) { return r; } - n->n_ref = 1; - n = NULL; + TAKE_PTR(n); bus_track_remove_from_queue(track); track->modified = true; @@ -258,20 +256,16 @@ _public_ int sd_bus_track_remove_name(sd_bus_track *track, const char *name) { if (!track) /* Treat a NULL track object as an empty track object */ return 0; - if (!track->recursive) - return bus_track_remove_name_fully(track, name); - i = hashmap_get(track->names, name); if (!i) - return -EUNATCH; - if (i->n_ref <= 0) - return -EUNATCH; + return 0; - i->n_ref--; - - if (i->n_ref <= 0) + assert(i->n_ref >= 1); + if (i->n_ref <= 1) return bus_track_remove_name_fully(track, name); + track_item_unref(i); + return 1; } @@ -293,7 +287,7 @@ _public_ const char* sd_bus_track_contains(sd_bus_track *track, const char *name if (!track) /* Let's consider a NULL object equivalent to an empty object */ return NULL; - return hashmap_get(track->names, (void*) name) ? name : NULL; + return hashmap_contains(track->names, name) ? name : NULL; } _public_ const char* sd_bus_track_first(sd_bus_track *track) { diff --git a/src/libsystemd/sd-bus/test-bus-track.c b/src/libsystemd/sd-bus/test-bus-track.c index 64aa88bb4f7..5604e84f522 100644 --- a/src/libsystemd/sd-bus/test-bus-track.c +++ b/src/libsystemd/sd-bus/test-bus-track.c @@ -10,6 +10,7 @@ static bool track_cb_called_x = false; static bool track_cb_called_y = false; +static bool track_destroy_called_z = false; static int track_cb_x(sd_bus_track *t, void *userdata) { @@ -26,7 +27,6 @@ static int track_cb_x(sd_bus_track *t, void *userdata) { } static int track_cb_y(sd_bus_track *t, void *userdata) { - int r; log_error("TRACK CB Y"); @@ -35,15 +35,22 @@ static int track_cb_y(sd_bus_track *t, void *userdata) { /* We got disconnected, let's close everything */ - r = sd_event_exit(sd_bus_get_event(sd_bus_track_get_bus(t)), EXIT_SUCCESS); - assert_se(r >= 0); + assert_se(sd_event_exit(sd_bus_get_event(sd_bus_track_get_bus(t)), EXIT_SUCCESS) >= 0); return 0; } +static int track_cb_z(sd_bus_track *t, void *userdata) { + assert_not_reached(); +} + +static void track_destroy_z(void *userdata) { + track_destroy_called_z = true; +} + int main(int argc, char *argv[]) { _cleanup_(sd_event_unrefp) sd_event *event = NULL; - _cleanup_(sd_bus_track_unrefp) sd_bus_track *x = NULL, *y = NULL; + _cleanup_(sd_bus_track_unrefp) sd_bus_track *x = NULL, *y = NULL, *z = NULL; _cleanup_(sd_bus_unrefp) sd_bus *a = NULL, *b = NULL; bool use_system_bus = false; const char *unique; @@ -51,8 +58,7 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_INFO); - r = sd_event_default(&event); - assert_se(r >= 0); + assert_se(sd_event_default(&event) >= 0); r = sd_bus_open_user(&a); if (IN_SET(r, -ECONNREFUSED, -ENOENT, -ENOMEDIUM)) { @@ -63,43 +69,80 @@ int main(int argc, char *argv[]) { } assert_se(r >= 0); - r = sd_bus_attach_event(a, event, SD_EVENT_PRIORITY_NORMAL); - assert_se(r >= 0); + assert_se(sd_bus_attach_event(a, event, SD_EVENT_PRIORITY_NORMAL) >= 0); if (use_system_bus) - r = sd_bus_open_system(&b); + assert_se(sd_bus_open_system(&b) >= 0); else - r = sd_bus_open_user(&b); - assert_se(r >= 0); + assert_se(sd_bus_open_user(&b) >= 0); - r = sd_bus_attach_event(b, event, SD_EVENT_PRIORITY_NORMAL); - assert_se(r >= 0); + assert_se(sd_bus_attach_event(b, event, SD_EVENT_PRIORITY_NORMAL) >= 0); /* Watch b's name from a */ - r = sd_bus_track_new(a, &x, track_cb_x, NULL); - assert_se(r >= 0); + assert_se(sd_bus_track_new(a, &x, track_cb_x, NULL) >= 0); - r = sd_bus_get_unique_name(b, &unique); - assert_se(r >= 0); + assert_se(sd_bus_get_unique_name(b, &unique) >= 0); - r = sd_bus_track_add_name(x, unique); - assert_se(r >= 0); + assert_se(sd_bus_track_add_name(x, unique) >= 0); /* Watch's a's own name from a */ - r = sd_bus_track_new(a, &y, track_cb_y, NULL); - assert_se(r >= 0); + assert_se(sd_bus_track_new(a, &y, track_cb_y, NULL) >= 0); - r = sd_bus_get_unique_name(a, &unique); - assert_se(r >= 0); + assert_se(sd_bus_get_unique_name(a, &unique) >= 0); - r = sd_bus_track_add_name(y, unique); - assert_se(r >= 0); + assert_se(sd_bus_track_add_name(y, unique) >= 0); + + /* Basic tests. */ + assert_se(sd_bus_track_new(a, &z, track_cb_z, NULL) >= 0); + + /* non-recursive case */ + assert_se(sd_bus_track_set_recursive(z, false) >= 0); + assert_se(sd_bus_track_get_recursive(z) == 0); + assert_se(!sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 0); + assert_se(sd_bus_track_remove_name(z, unique) == 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_set_recursive(z, true) == -EBUSY); + assert_se(sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 1); + assert_se(sd_bus_track_remove_name(z, unique) == 1); + assert_se(!sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 0); + assert_se(sd_bus_track_remove_name(z, unique) == 0); + + /* recursive case */ + assert_se(sd_bus_track_set_recursive(z, true) >= 0); + assert_se(sd_bus_track_get_recursive(z) == 1); + assert_se(!sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 0); + assert_se(sd_bus_track_remove_name(z, unique) == 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_add_name(z, unique) >= 0); + assert_se(sd_bus_track_set_recursive(z, false) == -EBUSY); + assert_se(sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 3); + assert_se(sd_bus_track_remove_name(z, unique) == 1); + assert_se(sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 2); + assert_se(sd_bus_track_remove_name(z, unique) == 1); + assert_se(sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 1); + assert_se(sd_bus_track_remove_name(z, unique) == 1); + assert_se(!sd_bus_track_contains(z, unique)); + assert_se(sd_bus_track_count_name(z, unique) == 0); + assert_se(sd_bus_track_remove_name(z, unique) == 0); + + assert_se(sd_bus_track_set_destroy_callback(z, track_destroy_z) >= 0); + z = sd_bus_track_unref(z); + assert_se(track_destroy_called_z); /* Now make b's name disappear */ sd_bus_close(b); - r = sd_event_loop(event); - assert_se(r >= 0); + assert_se(sd_event_loop(event) >= 0); assert_se(track_cb_called_x); assert_se(track_cb_called_y); diff --git a/src/shared/json.c b/src/shared/json.c index dff95eda265..f66b7df24ca 100644 --- a/src/shared/json.c +++ b/src/shared/json.c @@ -41,7 +41,7 @@ assert_cc(DEPTH_MAX <= UINT16_MAX); typedef struct JsonSource { /* When we parse from a file or similar, encodes the filename, to indicate the source of a json variant */ - size_t n_ref; + unsigned n_ref; unsigned max_line; unsigned max_column; char name[]; @@ -53,7 +53,7 @@ struct JsonVariant { /* We either maintain a reference counter for this variant itself, or we are embedded into an * array/object, in which case only that surrounding object is ref-counted. (If 'embedded' is false, * see below.) */ - size_t n_ref; + unsigned n_ref; /* If this JsonVariant is part of an array/object, then this field points to the surrounding * JSON_VARIANT_ARRAY/JSON_VARIANT_OBJECT object. (If 'embedded' is true, see below.) */