1
0
mirror of https://github.com/systemd/systemd.git synced 2025-03-19 22:50:17 +03:00

Merge pull request #23099 from yuwata/sd-bus-track-fixlets

sd-bus: fix counter
This commit is contained in:
Lennart Poettering 2022-04-20 18:23:05 +02:00 committed by GitHub
commit 947914cb44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 98 additions and 57 deletions

View File

@ -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; \
}

View File

@ -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) {

View File

@ -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);

View File

@ -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.) */