Merge pull request #18331 from yuwata/test-udev-event-spawn

udev: add tests for udev_event_spawn()

src/libsystemd/sd-event/sd-event.c

@@ -3781,8 +3781,8 @@ pending:
 }
 
 _public_ int sd_event_wait(sd_event *e, uint64_t timeout) {
-        size_t event_queue_max;
+        size_t n_event_queue, m;
-        int r, m, i;
+        int r, msec;
 
         assert_return(e, -EINVAL);
         assert_return(e = event_resolve(e), -ENOPKG);
@@ -3795,17 +3795,19 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) {
                 return 1;
         }
 
-        event_queue_max = MAX(e->n_sources, 1u);
+        n_event_queue = MAX(e->n_sources, 1u);
-        if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, event_queue_max))
+        if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, n_event_queue))
                 return -ENOMEM;
 
         /* If we still have inotify data buffered, then query the other fds, but don't wait on it */
         if (e->inotify_data_buffered)
-                timeout = 0;
+                msec = 0;
+        else
+                msec = timeout == (uint64_t) -1 ? -1 : (int) DIV_ROUND_UP(timeout, USEC_PER_MSEC);
 
-        m = epoll_wait(e->epoll_fd, e->event_queue, event_queue_max,
+        for (;;) {
-                       timeout == (uint64_t) -1 ? -1 : (int) DIV_ROUND_UP(timeout, USEC_PER_MSEC));
+                r = epoll_wait(e->epoll_fd, e->event_queue, e->event_queue_allocated, msec);
-        if (m < 0) {
+                if (r < 0) {
                         if (errno == EINTR) {
                                 e->state = SD_EVENT_PENDING;
                                 return 1;
@@ -3815,9 +3817,23 @@ _public_ int sd_event_wait(sd_event *e, uint64_t timeout) {
                         goto finish;
                 }
 
+                m = (size_t) r;
+
+                if (m < e->event_queue_allocated)
+                        break;
+
+                if (e->event_queue_allocated >= n_event_queue * 10)
+                        break;
+
+                if (!GREEDY_REALLOC(e->event_queue, e->event_queue_allocated, e->event_queue_allocated + n_event_queue))
+                        return -ENOMEM;
+
+                msec = 0;
+        }
+
         triple_timestamp_get(&e->timestamp);
 
-        for (i = 0; i < m; i++) {
+        for (size_t i = 0; i < m; i++) {
 
                 if (e->event_queue[i].data.ptr == INT_TO_PTR(SOURCE_WATCHDOG))
                         r = flush_timer(e, e->watchdog_fd, e->event_queue[i].events, NULL);

src/udev/meson.build

@@ -191,6 +191,12 @@ fuzzers += [
 ]
 
 tests += [
+        [['src/udev/test-udev-event.c'],
+         [libudevd_core,
+          libshared],
+         [threads,
+          libacl]],
+
         [['src/udev/fido_id/test-fido-id-desc.c',
           'src/udev/fido_id/fido_id_desc.c']],
 ]

src/udev/test-udev-event.c

@@ -0,0 +1,108 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+
+#include "path-util.h"
+#include "signal-util.h"
+#include "strv.h"
+#include "tests.h"
+#include "udev-event.h"
+#include "util.h"
+
+#define BUF_SIZE 1024
+
+static void test_event_spawn_core(bool with_pidfd, const char *cmd, char result_buf[BUF_SIZE]) {
+        _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
+        _cleanup_(udev_event_freep) UdevEvent *event = NULL;
+
+        assert_se(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1) >= 0);
+
+        assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net/lo") >= 0);
+        assert_se(event = udev_event_new(dev, 0, NULL, LOG_DEBUG));
+        assert_se(udev_event_spawn(event, 5 * USEC_PER_SEC, SIGKILL, false, cmd, result_buf, BUF_SIZE) == 0);
+
+        assert_se(unsetenv("SYSTEMD_PIDFD") >= 0);
+}
+
+static void test_event_spawn_cat(bool with_pidfd) {
+        _cleanup_strv_free_ char **lines = NULL;
+        _cleanup_free_ char *cmd = NULL;
+        char result_buf[BUF_SIZE];
+
+        log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd));
+
+        assert_se(find_executable("cat", &cmd) >= 0);
+        assert_se(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent"));
+
+        test_event_spawn_core(with_pidfd, cmd, result_buf);
+
+        assert_se(lines = strv_split_newlines(result_buf));
+        strv_print(lines);
+
+        assert_se(strv_contains(lines, "INTERFACE=lo"));
+        assert_se(strv_contains(lines, "IFINDEX=1"));
+}
+
+static void test_event_spawn_self(const char *self, const char *arg, bool with_pidfd) {
+        _cleanup_strv_free_ char **lines = NULL;
+        _cleanup_free_ char *cmd = NULL;
+        char result_buf[BUF_SIZE];
+
+        log_debug("/* %s(%s, %s) */", __func__, arg, yes_no(with_pidfd));
+
+        assert_se(cmd = strjoin(self, " ", arg));
+
+        test_event_spawn_core(with_pidfd, cmd, result_buf);
+
+        assert_se(lines = strv_split_newlines(result_buf));
+        strv_print(lines);
+
+        assert_se(strv_contains(lines, "aaa"));
+        assert_se(strv_contains(lines, "bbb"));
+}
+
+static void test1(void) {
+        fprintf(stdout, "aaa\nbbb");
+        fprintf(stderr, "ccc\nddd");
+}
+
+static void test2(void) {
+        char buf[16384];
+
+        fprintf(stdout, "aaa\nbbb");
+
+        memset(buf, 'a', sizeof(buf) - 1);
+        char_array_0(buf);
+        fputs(buf, stderr);
+}
+
+int main(int argc, char *argv[]) {
+        _cleanup_free_ char *self = NULL;
+
+        if (argc > 1) {
+                if (streq(argv[1], "test1"))
+                        test1();
+                else if (streq(argv[1], "test2"))
+                        test2();
+                else
+                        assert_not_reached("unknown command.");
+
+                return 0;
+        }
+
+        test_setup_logging(LOG_DEBUG);
+
+        assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGCHLD, -1) >= 0);
+
+        test_event_spawn_cat(true);
+        test_event_spawn_cat(false);
+
+        assert_se(path_make_absolute_cwd(argv[0], &self) >= 0);
+        path_simplify(self, true);
+
+        test_event_spawn_self(self, "test1", true);
+        test_event_spawn_self(self, "test1", false);
+
+        test_event_spawn_self(self, "test2", true);
+        test_event_spawn_self(self, "test2", false);
+
+        return 0;
+}

src/udev/udev-event.c

@@ -575,19 +575,19 @@ static int on_spawn_io(sd_event_source *s, int fd, uint32_t revents, void *userd
 
                 v = strv_split_newlines(p);
                 if (!v)
-                        return 0;
+                        log_oom_debug();
 
                 STRV_FOREACH(q, v)
                         log_device_debug(spawn->device, "'%s'(%s) '%s'", spawn->cmd,
                                          fd == spawn->fd_stdout ? "out" : "err", *q);
         }
 
-
         if (l == 0)
                 return 0;
 
-        /* Re-enable the event source if we did not encounter EOF */
 reenable:
+        /* Re-enable the event source if we did not encounter EOF */
+
         r = sd_event_source_set_enabled(s, SD_EVENT_ONESHOT);
         if (r < 0)
                 log_device_error_errno(spawn->device, r,
@@ -661,7 +661,7 @@ static int spawn_wait(Spawn *spawn) {
 
         r = sd_event_new(&e);
         if (r < 0)
-                return r;
+                return log_device_debug_errno(spawn->device, r, "Failed to allocate sd-event object: %m");
 
         if (spawn->timeout_usec > 0) {
                 usec_t usec, age_usec;
@@ -678,7 +678,7 @@ static int spawn_wait(Spawn *spawn) {
                                                       usec + spawn->timeout_warn_usec, USEC_PER_SEC,
                                                       on_spawn_timeout_warning, spawn);
                                 if (r < 0)
-                                        return r;
+                                        return log_device_debug_errno(spawn->device, r, "Failed to create timeout warning event source: %m");
                         }
 
                         spawn->timeout_usec -= age_usec;
@@ -686,35 +686,35 @@ static int spawn_wait(Spawn *spawn) {
                         r = sd_event_add_time(e, NULL, CLOCK_MONOTONIC,
                                               usec + spawn->timeout_usec, USEC_PER_SEC, on_spawn_timeout, spawn);
                         if (r < 0)
-                                return r;
+                                return log_device_debug_errno(spawn->device, r, "Failed to create timeout event source: %m");
                 }
         }
 
         if (spawn->fd_stdout >= 0) {
                 r = sd_event_add_io(e, &stdout_source, spawn->fd_stdout, EPOLLIN, on_spawn_io, spawn);
                 if (r < 0)
-                        return r;
+                        return log_device_debug_errno(spawn->device, r, "Failed to create stdio event source: %m");
                 r = sd_event_source_set_enabled(stdout_source, SD_EVENT_ONESHOT);
                 if (r < 0)
-                        return r;
+                        return log_device_debug_errno(spawn->device, r, "Failed to enable stdio event source: %m");
         }
 
         if (spawn->fd_stderr >= 0) {
                 r = sd_event_add_io(e, &stderr_source, spawn->fd_stderr, EPOLLIN, on_spawn_io, spawn);
                 if (r < 0)
-                        return r;
+                        return log_device_debug_errno(spawn->device, r, "Failed to create stderr event source: %m");
                 r = sd_event_source_set_enabled(stderr_source, SD_EVENT_ONESHOT);
                 if (r < 0)
-                        return r;
+                        return log_device_debug_errno(spawn->device, r, "Failed to enable stderr event source: %m");
         }
 
         r = sd_event_add_child(e, &sigchld_source, spawn->pid, WEXITED, on_spawn_sigchld, spawn);
         if (r < 0)
-                return r;
+                return log_device_debug_errno(spawn->device, r, "Failed to create sigchild event source: %m");
         /* SIGCHLD should be processed after IO is complete */
         r = sd_event_source_set_priority(sigchld_source, SD_EVENT_PRIORITY_NORMAL + 1);
         if (r < 0)
-                return r;
+                return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m");
 
         return sd_event_loop(e);
 }

src/udev/udev-rules.c

@@ -1730,7 +1730,8 @@ static int udev_rule_apply_token_to_event(
                 return token->op == OP_MATCH;
         }
         case TK_M_IMPORT_PROGRAM: {
-                char result[UDEV_LINE_SIZE], *line, *pos;
+                _cleanup_strv_free_ char **lines = NULL;
+                char result[UDEV_LINE_SIZE], **line;
 
                 (void) udev_event_apply_format(event, token->value, buf, sizeof(buf), false);
                 log_rule_debug(dev, rules, "Importing properties from results of '%s'", buf);
@@ -1744,18 +1745,18 @@ static int udev_rule_apply_token_to_event(
                         return token->op == OP_NOMATCH;
                 }
 
-                for (line = result; !isempty(line); line = pos) {
+                lines = strv_split_newlines(result);
+                if (!lines)
+                        return log_oom();
+
+                STRV_FOREACH(line, lines) {
                         char *key, *value;
 
-                        pos = strchr(line, '\n');
+                        r = get_property_from_string(*line, &key, &value);
-                        if (pos)
-                                *pos++ = '\0';
-
-                        r = get_property_from_string(line, &key, &value);
                         if (r < 0) {
                                 log_rule_debug_errno(dev, rules, r,
                                                      "Failed to parse key and value from '%s', ignoring: %m",
-                                                     line);
+                                                     *line);
                                 continue;
                         }
                         if (r == 0)