[systemd-devel] [PATCH] libudev: replace name_to_handle_at with normal sscanf

[Zbigniew Jędrzejewski-Szmek]

This has the advantage that we use the same sscanf
pattern as in other places where /proc/self/mountinfo is parsed,
and we avoid bugreports from people who are confused about missing
CONFIG_FHANDLE.

An alternate solution would be to warn when (at runtime) name_to_handle_at
is detected to be missing, but this is used in early boot, and seems
less useful overall.

https://bugzilla.redhat.com/show_bug.cgi?id=1072966
---
Hi Kay,
it seems that handles are not crucial, and the simplified version below
works too. Is there something I'm missing?

Zbyszek


 README                        |  2 +-
 src/libudev/libudev-monitor.c | 49 ++++++++++++++++++++++++-------------------
 src/shared/path-util.c        |  6 +-----
 3 files changed, 30 insertions(+), 27 deletions(-)

diff --git a/README b/README
index cecbcbf..983f02f 100644
--- a/README
+++ b/README
@@ -51,7 +51,6 @@ REQUIREMENTS:
           CONFIG_NET
           CONFIG_SYSFS
           CONFIG_PROC_FS
-          CONFIG_FHANDLE (libudev, mount and bind mount handling)
 
         Udev will fail to work with the legacy layout:
           CONFIG_SYSFS_DEPRECATED=n
@@ -79,6 +78,7 @@ REQUIREMENTS:
           CONFIG_TMPFS_POSIX_ACL
           CONFIG_TMPFS_XATTR
           CONFIG_SECCOMP
+          CONFIG_FHANDLE (libudev, mount and bind mount handling)
 
         For systemd-bootchart, several proc debug interfaces are required:
           CONFIG_SCHEDSTATS
diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c
index 3f7436b..0590002 100644
--- a/src/libudev/libudev-monitor.c
+++ b/src/libudev/libudev-monitor.c
@@ -108,39 +108,46 @@ static struct udev_monitor *udev_monitor_new(struct udev *udev)
 
 /* we consider udev running when /dev is on devtmpfs */
 static bool udev_has_devtmpfs(struct udev *udev) {
-        struct file_handle *h;
-        int mount_id;
         _cleanup_fclose_ FILE *f = NULL;
-        char line[LINE_MAX], *e;
-        int r;
-
-        h = alloca(MAX_HANDLE_SZ);
-        h->handle_bytes = MAX_HANDLE_SZ;
-        r = name_to_handle_at(AT_FDCWD, "/dev", h, &mount_id, 0);
-        if (r < 0)
-                return false;
-
+        int i;
 
         f = fopen("/proc/self/mountinfo", "re");
         if (!f)
                 return false;
 
-        FOREACH_LINE(line, f, return false) {
-                int mid;
-
-                if (sscanf(line, "%i", &mid) != 1)
-                        continue;
+        for (i = 1;; i++) {
+                _cleanup_free_ char *where = NULL, *fstype = NULL;
+                int k;
+
+                k = fscanf(f,
+                           "%*s "       /* (1) mount id */
+                           "%*s "       /* (2) parent id */
+                           "%*s "       /* (3) major:minor */
+                           "%*s "       /* (4) root */
+                           "%ms "       /* (5) mount point */
+                           "%*s"        /* (6) mount options */
+                           "%*[^-]"     /* (7) optional fields */
+                           "- "         /* (8) separator */
+                           "%ms "       /* (9) file system type */
+                           "%*s"        /* (10) mount source */
+                           "%*s"        /* (11) mount options 2 */
+                           "%*[^\n]",   /* some rubbish at the end */
+                           &where,
+                           &fstype);
+
+                if (k == EOF)
+                        break;
 
-                if (mid != mount_id)
+                if (k != 2) {
+                        log_warning("Failed to parse /proc/self/mountinfo:%u.", i);
                         continue;
+                }
 
-                e = strstr(line, " - ");
-                if (!e)
+                if (!streq(where, "/dev"))
                         continue;
 
                 /* accept any name that starts with the currently expected type */
-                if (startswith(e + 3, "devtmpfs"))
-                        return true;
+                return streq(fstype, "devtmpfs");
         }
 
         return false;
diff --git a/src/shared/path-util.c b/src/shared/path-util.c
index e35d7f8..5beb5f8 100644
--- a/src/shared/path-util.c
+++ b/src/shared/path-util.c
@@ -324,7 +324,7 @@ bool path_equal(const char *a, const char *b) {
 }
 
 int path_is_mount_point(const char *t, bool allow_symlink) {
-        char *parent;
+        _cleanup_free_ char *parent = NULL;
         int r;
         struct file_handle *h;
         int mount_id, mount_id_parent;
@@ -360,8 +360,6 @@ int path_is_mount_point(const char *t, bool allow_symlink) {
 
         h->handle_bytes = MAX_HANDLE_SZ;
         r = name_to_handle_at(AT_FDCWD, parent, h, &mount_id_parent, 0);
-        free(parent);
-
         if (r < 0) {
                 /* The parent can't do name_to_handle_at() but the
                  * directory we are interested in can? If so, it must
@@ -392,8 +390,6 @@ fallback:
                 return r;
 
         r = lstat(parent, &b);
-        free(parent);
-
         if (r < 0)
                 return -errno;
 
-- 
1.8.5.2