From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 4 Jun 2018 19:35:30 +0000 (+0200)
Subject: core: split out device validation from device_found_node()
X-Git-Tag: v239~123^2~12
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=34c20ee09fc0a83a8d08ee28518ac2b77dfe3373;p=thirdparty%2Fsystemd.git

core: split out device validation from device_found_node()

Let's separate the validate step out. Also, let's update some comments
which have long ceased to be true.

No change in behaviour.
---

diff --git a/src/core/device.c b/src/core/device.c
index 26a5014e282..e296ffa418b 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -941,56 +941,76 @@ static bool device_supported(void) {
         return read_only <= 0;
 }
 
-void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) {
-        _cleanup_(udev_device_unrefp) struct udev_device *dev = NULL;
+static int validate_node(Manager *m, const char *node, struct udev_device **ret) {
         struct stat st;
 
+        assert(m);
+        assert(node);
+        assert(ret);
+
+        /* Validates a device node that showed up in /proc/swaps or /proc/self/mountinfo if it makes sense for us to
+         * track. Note that this validator is fine within missing device nodes, but not with badly set up ones! */
+
+        if (!path_startswith(node, "/dev")) {
+                *ret = NULL;
+                return 0; /* bad! */
+        }
+
+        if (stat(node, &st) < 0) {
+                if (errno != ENOENT)
+                        return log_error_errno(errno, "Failed to stat() device node file %s: %m", node);
+
+                *ret = NULL;
+                return 1; /* good! (though missing) */
+
+        } else {
+                _cleanup_(udev_device_unrefp) struct udev_device *dev = NULL;
+
+                if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode)) {
+                        *ret = NULL;
+                        return 0; /* bad! */
+                }
+
+                dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev);
+                if (!dev) {
+                        if (errno != ENOENT)
+                                return log_error_errno(errno, "Failed to get udev device from devnum %u:%u: %m", major(st.st_rdev), minor(st.st_rdev));
+
+                        *ret = NULL;
+                        return 1; /* good! (though missing) */
+                }
+
+                *ret = TAKE_PTR(dev);
+                return 1; /* good! */
+        }
+}
+
+void device_found_node(Manager *m, const char *node, DeviceFound found, DeviceFound mask) {
+        int r;
+
         assert(m);
         assert(node);
 
         if (!device_supported())
                 return;
 
-        /* This is called whenever we find a device referenced in
-         * /proc/swaps or /proc/self/mounts. Such a device might be
-         * mounted/enabled at a time where udev has not finished
-         * probing it yet, and we thus haven't learned about it
-         * yet. In this case we will set the device unit to
-         * "tentative" state. */
+        if (mask == 0)
+                return;
+
+        /* This is called whenever we find a device referenced in /proc/swaps or /proc/self/mounts. Such a device might
+         * be mounted/enabled at a time where udev has not finished probing it yet, and we thus haven't learned about
+         * it yet. In this case we will set the device unit to "tentative" state. */
 
         if ((found & mask) != 0) {
-                if (!path_startswith(node, "/dev"))
-                        return;
-
-                /* We make an extra check here, if the device node
-                 * actually exists. If it's missing, then this is an
-                 * indication that device was unplugged but is still
-                 * referenced in /proc/swaps or
-                 * /proc/self/mountinfo. Note that this check doesn't
-                 * really cover all cases where a device might be gone
-                 * away, since drives that can have a medium inserted
-                 * will still have a device node even when the medium
-                 * is not there... */
-
-                if (stat(node, &st) >= 0) {
-                        if (!S_ISBLK(st.st_mode) && !S_ISCHR(st.st_mode))
-                                return;
-
-                        dev = udev_device_new_from_devnum(m->udev, S_ISBLK(st.st_mode) ? 'b' : 'c', st.st_rdev);
-                        if (!dev && errno != ENOENT) {
-                                log_error_errno(errno, "Failed to get udev device from devnum %u:%u: %m", major(st.st_rdev), minor(st.st_rdev));
-                                return;
-                        }
+                _cleanup_(udev_device_unrefp) struct udev_device *dev = NULL;
 
-                } else if (errno != ENOENT) {
-                        log_error_errno(errno, "Failed to stat device node file %s: %m", node);
-                        return;
-                }
+                /* If the device is known in the kernel and newly appeared, then we'll create a device unit for it,
+                 * under the name referenced in /proc/swaps or /proc/self/mountinfo. But first, let's validate if
+                 * everything is alright with the device node. */
 
-                /* If the device is known in the kernel and newly
-                 * appeared, then we'll create a device unit for it,
-                 * under the name referenced in /proc/swaps or
-                 * /proc/self/mountinfo. */
+                r = validate_node(m, node, &dev);
+                if (r <= 0)
+                        return; /* Don't create a device unit for this if the device node is borked. */
 
                 (void) device_setup_unit(m, dev, node, false);
         }