From 147d8fc1a7b9e16c3537588b700a9f3f97c40b08 Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sun, 10 May 2020 18:33:16 +0200 Subject: [PATCH 1/3] basic: Allow to call STRV_FOREACH_BACKWARDS() with a char * const * strv --- src/basic/strv.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/strv.h b/src/basic/strv.h index 0837e65a18..0f81e34580 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -106,7 +106,7 @@ bool strv_overlap(char * const *a, char * const *b) _pure_; #define STRV_FOREACH_BACKWARDS(s, l) \ for (s = ({ \ - char **_l = l; \ + typeof(l) _l = l; \ _l ? _l + strv_length(_l) - 1U : NULL; \ }); \ (l) && ((s) >= (l)); \ From fcee2755ec1fa3f02fb722ace068b0801c0aea1f Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sun, 10 May 2020 18:38:08 +0200 Subject: [PATCH 2/3] core: Update prototype of notify_message, tags list is read only Indicates that the tags list cannot be modified by notify_message function. Since the tags list is created only once for multiple call to notify_message functions. --- src/core/service.c | 8 ++++---- src/core/unit.h | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/service.c b/src/core/service.c index b3c4ad73e5..6e2a82a286 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3847,7 +3847,7 @@ static int service_dispatch_watchdog(sd_event_source *source, usec_t usec, void return 0; } -static bool service_notify_message_authorized(Service *s, pid_t pid, char **tags, FDSet *fds) { +static bool service_notify_message_authorized(Service *s, pid_t pid, FDSet *fds) { assert(s); if (s->notify_access == NOTIFY_NONE) { @@ -3894,19 +3894,19 @@ static void service_force_watchdog(Service *s) { static void service_notify_message( Unit *u, const struct ucred *ucred, - char **tags, + char * const *tags, FDSet *fds) { Service *s = SERVICE(u); bool notify_dbus = false; const char *e; - char **i; + char * const *i; int r; assert(u); assert(ucred); - if (!service_notify_message_authorized(SERVICE(u), ucred->pid, tags, fds)) + if (!service_notify_message_authorized(SERVICE(u), ucred->pid, fds)) return; if (DEBUG_LOGGING) { diff --git a/src/core/unit.h b/src/core/unit.h index a4c342a396..4ad429bf30 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -539,7 +539,7 @@ typedef struct UnitVTable { void (*notify_cgroup_oom)(Unit *u); /* Called whenever a process of this unit sends us a message */ - void (*notify_message)(Unit *u, const struct ucred *ucred, char **tags, FDSet *fds); + void (*notify_message)(Unit *u, const struct ucred *ucred, char * const *tags, FDSet *fds); /* Called whenever a name this Unit registered for comes or goes away. */ void (*bus_name_owner_change)(Unit *u, const char *new_owner); From 5151b4ccd2387f144383dbcc9c07a374f719ccb6 Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sun, 10 May 2020 18:46:45 +0200 Subject: [PATCH 3/3] core: Parse the tags list sooner, and use it for multiple function - Parse the tags list using strv_split_newlines() which remove any unnecessary empty string at the end of the strv. - Use this parsed list for manager_process_barrier_fd() and every call to manager_invoke_notify_message(). - This also allow to simplify the manager_process_barrier_fd() function. --- src/core/manager.c | 55 ++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/src/core/manager.c b/src/core/manager.c index 09dc92d599..1beab6baed 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -2288,16 +2288,19 @@ static int manager_dispatch_cgroups_agent_fd(sd_event_source *source, int fd, ui return 0; } -static bool manager_process_barrier_fd(const char *buf, FDSet *fds) { - assert(buf); +static bool manager_process_barrier_fd(char * const *tags, FDSet *fds) { /* nothing else must be sent when using BARRIER=1 */ - if (STR_IN_SET(buf, "BARRIER=1", "BARRIER=1\n")) { - if (fdset_size(fds) != 1) - log_warning("Got incorrect number of fds with BARRIER=1, closing them."); + if (strv_contains(tags, "BARRIER=1")) { + if (strv_length(tags) == 1) { + if (fdset_size(fds) != 1) + log_warning("Got incorrect number of fds with BARRIER=1, closing them."); + } else + log_warning("Extra notification messages sent with BARRIER=1, ignoring everything."); + + /* Drop the message if BARRIER=1 was found */ return true; - } else if (startswith(buf, "BARRIER=1\n") || strstr(buf, "\nBARRIER=1\n") || endswith(buf, "\nBARRIER=1")) - log_warning("Extra notification messages sent with BARRIER=1, ignoring everything."); + } return false; } @@ -2306,33 +2309,27 @@ static void manager_invoke_notify_message( Manager *m, Unit *u, const struct ucred *ucred, - const char *buf, + char * const *tags, FDSet *fds) { assert(m); assert(u); assert(ucred); - assert(buf); + assert(tags); if (u->notifygen == m->notifygen) /* Already invoked on this same unit in this same iteration? */ return; u->notifygen = m->notifygen; - if (UNIT_VTABLE(u)->notify_message) { - _cleanup_strv_free_ char **tags = NULL; - - tags = strv_split(buf, NEWLINE); - if (!tags) { - log_oom(); - return; - } - + if (UNIT_VTABLE(u)->notify_message) UNIT_VTABLE(u)->notify_message(u, ucred, tags, fds); - } else if (DEBUG_LOGGING) { - _cleanup_free_ char *x = NULL, *y = NULL; + else if (DEBUG_LOGGING) { + _cleanup_free_ char *buf = NULL, *x = NULL, *y = NULL; - x = ellipsize(buf, 20, 90); + buf = strv_join(tags, ", "); + if (buf) + x = ellipsize(buf, 20, 90); if (x) y = cescape(x); @@ -2361,6 +2358,7 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t struct cmsghdr *cmsg; struct ucred *ucred = NULL; _cleanup_free_ Unit **array_copy = NULL; + _cleanup_strv_free_ char **tags = NULL; Unit *u1, *u2, **array; int r, *fd_array = NULL; size_t n_fds = 0; @@ -2429,11 +2427,16 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t return 0; } - /* Make sure it's NUL-terminated. */ + /* Make sure it's NUL-terminated, then parse it to obtain the tags list */ buf[n] = 0; + tags = strv_split_newlines(buf); + if (!tags) { + log_oom(); + return 0; + } /* possibly a barrier fd, let's see */ - if (manager_process_barrier_fd(buf, fds)) + if (manager_process_barrier_fd(tags, fds)) return 0; /* Increase the generation counter used for filtering out duplicate unit invocations. */ @@ -2456,16 +2459,16 @@ static int manager_dispatch_notify_fd(sd_event_source *source, int fd, uint32_t /* And now invoke the per-unit callbacks. Note that manager_invoke_notify_message() will handle duplicate units * make sure we only invoke each unit's handler once. */ if (u1) { - manager_invoke_notify_message(m, u1, ucred, buf, fds); + manager_invoke_notify_message(m, u1, ucred, tags, fds); found = true; } if (u2) { - manager_invoke_notify_message(m, u2, ucred, buf, fds); + manager_invoke_notify_message(m, u2, ucred, tags, fds); found = true; } if (array_copy) for (size_t i = 0; array_copy[i]; i++) { - manager_invoke_notify_message(m, array_copy[i], ucred, buf, fds); + manager_invoke_notify_message(m, array_copy[i], ucred, tags, fds); found = true; }