sd-event: fix sd_event_source_get_inotify_path()

Follow-ups for 74c4231ce5f6cddabc2500391a8d5fd69e89b79e.

Previously, the path is obtained from the fd, but it is closed in
sd_event_loop() to unpin the filesystem.
So, let's save the path when the event source is created, and make
sd_event_source_get_inotify_path() simply read it.

diff --git a/man/sd_event_add_inotify.xml b/man/sd_event_add_inotify.xml
index 636f37b0012fc2004a362bb6e6e1fad81b17c1aa..ed26c8ac96ecf1deab7afe7fd69dab1cb0a168f9 100644 (file)
--- a/man/sd_event_add_inotify.xml
+++ b/man/sd_event_add_inotify.xml
@@ -67,7 +67,7 @@
       <funcprototype>
         <funcdef>int <function>sd_event_source_get_inotify_path</function></funcdef>
         <paramdef>sd_event_source *<parameter>source</parameter></paramdef>
-        <paramdef>char **<parameter>ret</parameter></paramdef>
+        <paramdef>const char **<parameter>ret</parameter></paramdef>
       </funcprototype>
 
     </funcsynopsis>
@@ -144,7 +144,8 @@
     <para><function>sd_event_source_get_inotify_path()</function> retrieves the target path of the configured
     inotify watch of an event source created previously with <function>sd_event_add_inotify()</function>. It
     takes the event source object as the <parameter>source</parameter> parameter and a pointer to a
-    <type>char **</type> variable to return the path in. The caller needs to free the returned path.</para>
+    <type>const char **</type> variable to return the path in. The caller must not free the returned path.
+    </para>
   </refsect1>
 
   <refsect1>
@@ -207,8 +208,11 @@
         <varlistentry>
           <term><constant>-ENOSYS</constant></term>
 
-          <listitem><para><function>sd_event_add_inotify_fd()</function> was called without
-          <filename>/proc/</filename> mounted.</para></listitem>
+          <listitem>
+            <para><function>sd_event_add_inotify_fd()</function> or
+            <function>sd_event_source_get_inotify_path()</function> was called without
+            <filename>/proc/</filename> mounted.</para>
+          </listitem>
 
         </varlistentry>
 

diff --git a/src/libsystemd/sd-event/event-source.h b/src/libsystemd/sd-event/event-source.h
index f4e38d78d085b847334df478042697ecbc4b9023..d05bcf0538df633cf97ad0f1d074ec7c2b25f0d8 100644 (file)
--- a/src/libsystemd/sd-event/event-source.h
+++ b/src/libsystemd/sd-event/event-source.h
@@ -189,6 +189,9 @@ struct inode_data {
          * iteration. */
         int fd;
 
+        /* The path that the fd points to. The field is optional. */
+        char *path;
+
         /* The inotify "watch descriptor" */
         int wd;
 

diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 23d9dc10c0f60863b5663465c2ea5a6f333afebe..39c60297f06280e11f15c44571dcaae4181d7238 100644 (file)
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2272,6 +2272,7 @@ static void event_free_inode_data(
                 assert_se(hashmap_remove(d->inotify_data->inodes, d) == d);
         }
 
+        free(d->path);
         free(d);
 }
 
@@ -2512,6 +2513,15 @@ static int event_add_inotify_fd_internal(
                 }
 
                 LIST_PREPEND(to_close, e->inode_data_to_close_list, inode_data);
+
+                _cleanup_free_ char *path = NULL;
+                r = fd_get_path(inode_data->fd, &path);
+                if (r < 0 && r != -ENOSYS) { /* The path is optional, hence ignore -ENOSYS. */
+                        event_gc_inode_data(e, inode_data);
+                        return r;
+                }
+
+                free_and_replace(inode_data->path, path);
         }
 
         /* Link our event source to the inode data object */
@@ -2798,6 +2808,13 @@ _public_ int sd_event_source_set_priority(sd_event_source *s, int64_t priority)
                         }
 
                         LIST_PREPEND(to_close, s->event->inode_data_to_close_list, new_inode_data);
+
+                        _cleanup_free_ char *path = NULL;
+                        r = fd_get_path(new_inode_data->fd, &path);
+                        if (r < 0 && r != -ENOSYS)
+                                goto fail;
+
+                        free_and_replace(new_inode_data->path, path);
                 }
 
                 /* Move the event source to the new inode data structure */
@@ -3292,16 +3309,20 @@ _public_ int sd_event_source_get_inotify_mask(sd_event_source *s, uint32_t *ret)
         return 0;
 }
 
-_public_ int sd_event_source_get_inotify_path(sd_event_source *s, char **ret) {
+_public_ int sd_event_source_get_inotify_path(sd_event_source *s, const char **ret) {
         assert_return(s, -EINVAL);
         assert_return(ret, -EINVAL);
         assert_return(s->type == SOURCE_INOTIFY, -EDOM);
         assert_return(!event_origin_changed(s->event), -ECHILD);
 
-        if (!s->inotify.inode_data || s->inotify.inode_data->fd < 0)
-                return -ESTALE;
+        if (!s->inotify.inode_data)
+                return -ESTALE; /* already disconnected. */
 
-        return fd_get_path(s->inotify.inode_data->fd, ret);
+        if (!s->inotify.inode_data->path)
+                return -ENOSYS; /* /proc was not mounted? */
+
+        *ret = s->inotify.inode_data->path;
+        return 0;
 }
 
 _public_ int sd_event_source_set_prepare(sd_event_source *s, sd_event_handler_t callback) {

diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c
index 87f704ecb190fb9e150a4f1c79abca71c6890f2b..991aa925a42a7d125ac1b9da386fbe099fb1ecba 100644 (file)
--- a/src/libsystemd/sd-event/test-event.c
+++ b/src/libsystemd/sd-event/test-event.c
@@ -396,6 +396,7 @@ struct inotify_context {
         unsigned create_called[CREATE_EVENTS_MAX];
         unsigned create_overflow;
         unsigned n_create_events;
+        const char *path;
 };
 
 static void maybe_exit(sd_event_source *s, struct inotify_context *c) {
@@ -422,10 +423,12 @@ static void maybe_exit(sd_event_source *s, struct inotify_context *c) {
 }
 
 static int inotify_handler(sd_event_source *s, const struct inotify_event *ev, void *userdata) {
-        struct inotify_context *c = userdata;
-        const char *description;
+        struct inotify_context *c = ASSERT_PTR(userdata);
+        const char *path, *description;
         unsigned bit, n;
 
+        assert_se(sd_event_source_get_inotify_path(s, &path) >= 0);
+
         assert_se(sd_event_source_get_description(s, &description) >= 0);
         assert_se(safe_atou(description, &n) >= 0);
 
@@ -433,11 +436,12 @@ static int inotify_handler(sd_event_source *s, const struct inotify_event *ev, v
         bit = 1U << n;
 
         if (ev->mask & IN_Q_OVERFLOW) {
-                log_info("inotify-handler <%s>: overflow", description);
+                log_info("inotify-handler for %s <%s>: overflow", path, description);
                 c->create_overflow |= bit;
         } else if (ev->mask & IN_CREATE) {
+                assert_se(path_equal_or_inode_same(path, c->path, 0));
                 if (streq(ev->name, "sub"))
-                        log_debug("inotify-handler <%s>: create on %s", description, ev->name);
+                        log_debug("inotify-handler for %s <%s>: create on %s", path, description, ev->name);
                 else {
                         unsigned i;
 
@@ -446,7 +450,7 @@ static int inotify_handler(sd_event_source *s, const struct inotify_event *ev, v
                         c->create_called[i] |= bit;
                 }
         } else if (ev->mask & IN_DELETE) {
-                log_info("inotify-handler <%s>: delete of %s", description, ev->name);
+                log_info("inotify-handler for %s <%s>: delete of %s", path, description, ev->name);
                 assert_se(streq(ev->name, "sub"));
         } else
                 assert_not_reached();
@@ -456,16 +460,19 @@ static int inotify_handler(sd_event_source *s, const struct inotify_event *ev, v
 }
 
 static int delete_self_handler(sd_event_source *s, const struct inotify_event *ev, void *userdata) {
-        struct inotify_context *c = userdata;
+        struct inotify_context *c = ASSERT_PTR(userdata);
+        const char *path;
+
+        assert_se(sd_event_source_get_inotify_path(s, &path) >= 0);
 
         if (ev->mask & IN_Q_OVERFLOW) {
-                log_info("delete-self-handler: overflow");
+                log_info("delete-self-handler for %s: overflow", path);
                 c->delete_self_handler_called = true;
         } else if (ev->mask & IN_DELETE_SELF) {
-                log_info("delete-self-handler: delete-self");
+                log_info("delete-self-handler for %s: delete-self", path);
                 c->delete_self_handler_called = true;
         } else if (ev->mask & IN_IGNORED) {
-                log_info("delete-self-handler: ignore");
+                log_info("delete-self-handler for %s: ignore", path);
         } else
                 assert_not_reached();
 
@@ -475,13 +482,12 @@ static int delete_self_handler(sd_event_source *s, const struct inotify_event *e
 
 static void test_inotify_one(unsigned n_create_events) {
         _cleanup_(rm_rf_physical_and_freep) char *p = NULL;
-        _cleanup_free_ char *pp = NULL;
         sd_event_source *a = NULL, *b = NULL, *c = NULL, *d = NULL;
         struct inotify_context context = {
                 .n_create_events = n_create_events,
         };
         sd_event *e = NULL;
-        const char *q;
+        const char *q, *pp;
         unsigned i;
 
         log_info("/* %s(%u) */", __func__, n_create_events);
@@ -489,6 +495,7 @@ static void test_inotify_one(unsigned n_create_events) {
         assert_se(sd_event_default(&e) >= 0);
 
         assert_se(mkdtemp_malloc("/tmp/test-inotify-XXXXXX", &p) >= 0);
+        context.path = p;
 
         assert_se(sd_event_add_inotify(e, &a, p, IN_CREATE|IN_ONLYDIR, inotify_handler, &context) >= 0);
         assert_se(sd_event_add_inotify(e, &b, p, IN_CREATE|IN_DELETE|IN_DONT_FOLLOW, inotify_handler, &context) >= 0);
@@ -503,13 +510,10 @@ static void test_inotify_one(unsigned n_create_events) {
 
         assert_se(sd_event_source_get_inotify_path(a, &pp) >= 0);
         assert_se(path_equal_or_inode_same(pp, p, 0));
-        pp = mfree(pp);
         assert_se(sd_event_source_get_inotify_path(b, &pp) >= 0);
         assert_se(path_equal_or_inode_same(pp, p, 0));
-        pp = mfree(pp);
         assert_se(sd_event_source_get_inotify_path(b, &pp) >= 0);
         assert_se(path_equal_or_inode_same(pp, p, 0));
-        pp = mfree(pp);
 
         q = strjoina(p, "/sub");
         assert_se(touch(q) >= 0);

diff --git a/src/systemd/sd-event.h b/src/systemd/sd-event.h
index e306c891cb13f50cf307a636a831a63ef1872f04..a876add00c12c681711a270bafd71c0df1a059a7 100644 (file)
--- a/src/systemd/sd-event.h
+++ b/src/systemd/sd-event.h
@@ -161,7 +161,7 @@ int sd_event_source_send_child_signal(sd_event_source *s, int sig, const siginfo
 int sd_event_source_send_child_signal(sd_event_source *s, int sig, const void *si, unsigned flags);
 #endif
 int sd_event_source_get_inotify_mask(sd_event_source *s, uint32_t *ret);
-int sd_event_source_get_inotify_path(sd_event_source *s, char **ret);
+int sd_event_source_get_inotify_path(sd_event_source *s, const char **ret);
 int sd_event_source_set_memory_pressure_type(sd_event_source *e, const char *ty);
 int sd_event_source_set_memory_pressure_period(sd_event_source *s, uint64_t threshold_usec, uint64_t window_usec);
 int sd_event_source_set_destroy_callback(sd_event_source *s, sd_event_destroy_t callback);