From f7c1ad4fd4190bee32db0aa26c8e9fe7e19d8816 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 10 Dec 2014 01:45:43 +0100 Subject: [PATCH] core: unify how we iterate over inotify events Let's add some syntactic sugar for iterating through inotify events, and use it everywhere. --- src/core/mount.c | 48 +++++++++++++++++++++++++--------------- src/core/path.c | 32 +++++++-------------------- src/journal/sd-journal.c | 14 ++---------- src/shared/util.c | 19 ++++------------ src/shared/util.h | 7 ++++++ src/udev/udevd.c | 33 +++++++++++---------------- 6 files changed, 64 insertions(+), 89 deletions(-) diff --git a/src/core/mount.c b/src/core/mount.c index 5f268c6dd..66de85b57 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1616,14 +1616,18 @@ static int mount_enumerate(Manager *m) { if (m->utab_inotify_fd < 0) { m->utab_inotify_fd = inotify_init1(IN_NONBLOCK|IN_CLOEXEC); - if (m->utab_inotify_fd < 0) - goto fail_with_errno; + if (m->utab_inotify_fd < 0) { + r = -errno; + goto fail; + } (void) mkdir_p_label("/run/mount", 0755); r = inotify_add_watch(m->utab_inotify_fd, "/run/mount", IN_MOVED_TO); - if (r < 0) - goto fail_with_errno; + if (r < 0) { + r = -errno; + goto fail; + } r = sd_event_add_io(m->event, &m->mount_utab_event_source, m->utab_inotify_fd, EPOLLIN, mount_dispatch_io, m); if (r < 0) @@ -1640,8 +1644,6 @@ static int mount_enumerate(Manager *m) { return 0; -fail_with_errno: - r = -errno; fail: mount_shutdown(m); return r; @@ -1661,22 +1663,32 @@ static int mount_dispatch_io(sd_event_source *source, int fd, uint32_t revents, * for mount options. */ if (fd == m->utab_inotify_fd) { - char inotify_buffer[sizeof(struct inotify_event) + NAME_MAX + 1]; - struct inotify_event *event; - char *p; bool rescan = false; - while ((r = read(fd, inotify_buffer, sizeof(inotify_buffer))) > 0) - for (p = inotify_buffer; p < inotify_buffer + r; ) { - event = (struct inotify_event *) p; - /* only care about changes to utab, but we have - * to monitor the directory to reliably get - * notifications about when utab is replaced - * using rename(2) */ - if ((event->mask & IN_Q_OVERFLOW) || streq(event->name, "utab")) + for (;;) { + uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event); + struct inotify_event *e; + ssize_t l; + + l = read(fd, buffer, sizeof(buffer)); + if (l < 0) { + if (errno == EAGAIN || errno == EINTR) + break; + + log_error_errno(errno, "Failed to read utab inotify: %m"); + break; + } + + FOREACH_INOTIFY_EVENT(e, buffer, l) { + /* Only care about changes to utab, + * but we have to monitor the + * directory to reliably get + * notifications about when utab is + * replaced using rename(2) */ + if ((e->mask & IN_Q_OVERFLOW) || streq(e->name, "utab")) rescan = true; - p += sizeof(struct inotify_event) + event->len; } + } if (!rescan) return 0; diff --git a/src/core/path.c b/src/core/path.c index 3624bfcac..656ed6941 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -157,10 +157,9 @@ void path_spec_unwatch(PathSpec *s) { } int path_spec_fd_event(PathSpec *s, uint32_t revents) { - _cleanup_free_ uint8_t *buf = NULL; + uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event); struct inotify_event *e; - ssize_t k; - int l; + ssize_t l; int r = 0; if (revents != EPOLLIN) { @@ -168,33 +167,18 @@ int path_spec_fd_event(PathSpec *s, uint32_t revents) { return -EINVAL; } - if (ioctl(s->inotify_fd, FIONREAD, &l) < 0) - return log_error_errno(errno, "FIONREAD failed: %m"); + l = read(s->inotify_fd, buffer, sizeof(buffer)); + if (l < 0) { + if (errno == EAGAIN || errno == EINTR) + return 0; - assert(l > 0); - - buf = malloc(l); - if (!buf) - return log_oom(); - - k = read(s->inotify_fd, buf, l); - if (k < 0) return log_error_errno(errno, "Failed to read inotify event: %m"); + } - e = (struct inotify_event*) buf; - - while (k > 0) { - size_t step; - + FOREACH_INOTIFY_EVENT(e, buffer, l) { if ((s->type == PATH_CHANGED || s->type == PATH_MODIFIED) && s->primary_wd == e->wd) r = 1; - - step = sizeof(struct inotify_event) + e->len; - assert(step <= (size_t) k); - - e = (struct inotify_event*) ((uint8_t*) e + step); - k -= step; } return r; diff --git a/src/journal/sd-journal.c b/src/journal/sd-journal.c index 5e2da9912..23aad740d 100644 --- a/src/journal/sd-journal.c +++ b/src/journal/sd-journal.c @@ -2320,7 +2320,6 @@ static int determine_change(sd_journal *j) { } _public_ int sd_journal_process(sd_journal *j) { - uint8_t buffer[sizeof(struct inotify_event) + FILENAME_MAX] _alignas_(struct inotify_event); bool got_something = false; assert_return(j, -EINVAL); @@ -2329,6 +2328,7 @@ _public_ int sd_journal_process(sd_journal *j) { j->last_process_usec = now(CLOCK_MONOTONIC); for (;;) { + uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event); struct inotify_event *e; ssize_t l; @@ -2342,18 +2342,8 @@ _public_ int sd_journal_process(sd_journal *j) { got_something = true; - e = (struct inotify_event*) buffer; - while (l > 0) { - size_t step; - + FOREACH_INOTIFY_EVENT(e, buffer, l) process_inotify_event(j, e); - - step = sizeof(struct inotify_event) + e->len; - assert(step <= (size_t) l); - - e = (struct inotify_event*) ((uint8_t*) e + step); - l -= step; - } } } diff --git a/src/shared/util.c b/src/shared/util.c index 8c1cf52c0..ff8835b72 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -2100,9 +2100,9 @@ int acquire_terminal( assert(notify >= 0); for (;;) { - uint8_t inotify_buffer[sizeof(struct inotify_event) + FILENAME_MAX]; - ssize_t l; + uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event); struct inotify_event *e; + ssize_t l; if (timeout != USEC_INFINITY) { usec_t n; @@ -2123,9 +2123,8 @@ int acquire_terminal( } } - l = read(notify, inotify_buffer, sizeof(inotify_buffer)); + l = read(notify, buffer, sizeof(buffer)); if (l < 0) { - if (errno == EINTR || errno == EAGAIN) continue; @@ -2133,21 +2132,11 @@ int acquire_terminal( goto fail; } - e = (struct inotify_event*) inotify_buffer; - - while (l > 0) { - size_t step; - + FOREACH_INOTIFY_EVENT(e, buffer, l) { if (e->wd != wd || !(e->mask & IN_CLOSE)) { r = -EIO; goto fail; } - - step = sizeof(struct inotify_event) + e->len; - assert(step <= (size_t) l); - - e = (struct inotify_event*) ((uint8_t*) e + step); - l -= step; } break; diff --git a/src/shared/util.h b/src/shared/util.h index b6fdf8330..61094cca2 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -1031,3 +1031,10 @@ int unquote_many_words(const char **p, ...) _sentinel_; int free_and_strdup(char **p, const char *s); int sethostname_idempotent(const char *s); + +#define INOTIFY_EVENT_MAX (sizeof(struct inotify_event) + NAME_MAX + 1) + +#define FOREACH_INOTIFY_EVENT(e, buffer, sz) \ + for ((e) = (struct inotify_event*) (buffer); \ + (uint8_t*) (e) < (uint8_t*) (buffer) + (sz); \ + (e) = (struct inotify_event*) ((uint8_t*) (e) + sizeof(struct inotify_event) + (e)->len)) diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 6a8dda327..8bec03e77 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -816,41 +816,34 @@ static int synthesize_change(struct udev_device *dev) { } static int handle_inotify(struct udev *udev) { - int nbytes, pos; - char *buf; - struct inotify_event *ev; - int r; + uint8_t buffer[INOTIFY_EVENT_MAX] _alignas_(struct inotify_event); + struct inotify_event *e; + ssize_t l; - r = ioctl(fd_inotify, FIONREAD, &nbytes); - if (r < 0 || nbytes <= 0) - return -errno; + l = read(fd_inotify, buffer, sizeof(buffer)); + if (l < 0) { + if (errno == EAGAIN || errno == EINTR) + return 0; - buf = malloc(nbytes); - if (!buf) { - log_error("error getting buffer for inotify"); - return -ENOMEM; + return log_error_errno(errno, "Failed to read inotify fd: %m"); } - nbytes = read(fd_inotify, buf, nbytes); - - for (pos = 0; pos < nbytes; pos += sizeof(struct inotify_event) + ev->len) { + FOREACH_INOTIFY_EVENT(e, buffer, l) { struct udev_device *dev; - ev = (struct inotify_event *)(buf + pos); - dev = udev_watch_lookup(udev, ev->wd); + dev = udev_watch_lookup(udev, e->wd); if (!dev) continue; - log_debug("inotify event: %x for %s", ev->mask, udev_device_get_devnode(dev)); - if (ev->mask & IN_CLOSE_WRITE) + log_debug("inotify event: %x for %s", e->mask, udev_device_get_devnode(dev)); + if (e->mask & IN_CLOSE_WRITE) synthesize_change(dev); - else if (ev->mask & IN_IGNORED) + else if (e->mask & IN_IGNORED) udev_watch_end(udev, dev); udev_device_unref(dev); } - free(buf); return 0; } -- 2.30.2