Merge changes Ie5ec609a,I5a2ac369,I690137b5

* changes:
  ueventd: Fix up string handling in handle_*_device_event()
  ueventd: convert mkdir_recursive() to std::string
  ueventd: move subsystem logic from code to ueventd.rc

init/Android.mk

@@ -148,8 +148,9 @@ LOCAL_SRC_FILES := \
     util_test.cpp \
 
 LOCAL_SHARED_LIBRARIES += \
-    libcutils \
     libbase \
+    libcutils \
+    libselinux \
 
 LOCAL_STATIC_LIBRARIES := libinit
 LOCAL_SANITIZE := integer

init/devices.cpp

@@ -604,94 +604,35 @@ static void handle_platform_device_event(struct uevent *uevent)
         remove_platform_device(path);
 }
 
-static const char *parse_device_name(struct uevent *uevent, unsigned int len)
-{
-    const char *name;
+static void handle_block_device_event(uevent* uevent) {
+    // if it's not a /dev device, nothing to do
+    if (uevent->major < 0 || uevent->minor < 0) return;
 
-    /* if it's not a /dev device, nothing else to do */
-    if((uevent->major < 0) || (uevent->minor < 0))
-        return NULL;
-
-    /* do we have a name? */
-    name = strrchr(uevent->path, '/');
-    if(!name)
-        return NULL;
-    name++;
-
-    /* too-long names would overrun our buffer */
-    if(strlen(name) > len) {
-        LOG(ERROR) << "DEVPATH=" << name << " exceeds " << len << "-character limit on filename; ignoring event";
-        return NULL;
-    }
-
-    return name;
-}
-
-#define DEVPATH_LEN 96
-#define MAX_DEV_NAME 64
-
-static void handle_block_device_event(struct uevent *uevent)
-{
-    const char *base = "/dev/block/";
-    const char *name;
-    char devpath[DEVPATH_LEN];
-
-    name = parse_device_name(uevent, MAX_DEV_NAME);
-    if (!name)
-        return;
-
-    snprintf(devpath, sizeof(devpath), "%s%s", base, name);
+    const char* base = "/dev/block/";
     make_dir(base, 0755);
 
+    std::string name = android::base::Basename(uevent->path);
+    std::string devpath = base + name;
+
     std::vector<std::string> links;
     if (!strncmp(uevent->path, "/devices/", 9))
         links = get_block_device_symlinks(uevent);
 
-    handle_device(uevent->action, devpath, uevent->path, 1,
-            uevent->major, uevent->minor, links);
+    handle_device(uevent->action, devpath.c_str(), uevent->path, 1, uevent->major, uevent->minor,
+                  links);
 }
 
-static bool assemble_devpath(char *devpath, const char *dirname,
-        const char *devname)
-{
-    int s = snprintf(devpath, DEVPATH_LEN, "%s/%s", dirname, devname);
-    if (s < 0) {
-        PLOG(ERROR) << "failed to assemble device path; ignoring event";
-        return false;
-    } else if (s >= DEVPATH_LEN) {
-        LOG(ERROR) << dirname << "/" << devname
-                   << " exceeds " << DEVPATH_LEN << "-character limit on path; ignoring event";
-        return false;
-    }
-    return true;
-}
+static void handle_generic_device_event(uevent* uevent) {
+    // if it's not a /dev device, nothing to do
+    if (uevent->major < 0 || uevent->minor < 0) return;
 
-static void mkdir_recursive_for_devpath(const char *devpath)
-{
-    char dir[DEVPATH_LEN];
-    char *slash;
+    std::string name = android::base::Basename(uevent->path);
+    ueventd_subsystem* subsystem = ueventd_subsystem_find_by_name(uevent->subsystem);
 
-    strcpy(dir, devpath);
-    slash = strrchr(dir, '/');
-    *slash = '\0';
-    mkdir_recursive(dir, 0755);
-}
-
-static void handle_generic_device_event(struct uevent *uevent)
-{
-    const char *base;
-    const char *name;
-    char devpath[DEVPATH_LEN] = {0};
-
-    name = parse_device_name(uevent, MAX_DEV_NAME);
-    if (!name)
-        return;
-
-    struct ueventd_subsystem *subsystem =
-            ueventd_subsystem_find_by_name(uevent->subsystem);
+    std::string devpath;
 
     if (subsystem) {
-        const char *devname;
+        std::string devname;
 
         switch (subsystem->devname_src) {
         case DEVNAME_UEVENT_DEVNAME:
@@ -707,67 +648,35 @@ static void handle_generic_device_event(struct uevent *uevent)
             return;
         }
 
-        if (!assemble_devpath(devpath, subsystem->dirname, devname))
-            return;
-        mkdir_recursive_for_devpath(devpath);
+        // TODO: Remove std::string()
+        devpath = std::string(subsystem->dirname) + "/" + devname;
+        mkdir_recursive(android::base::Dirname(devpath), 0755);
     } else if (!strncmp(uevent->subsystem, "usb", 3)) {
-         if (!strcmp(uevent->subsystem, "usb")) {
+        if (!strcmp(uevent->subsystem, "usb")) {
             if (uevent->device_name) {
-                if (!assemble_devpath(devpath, "/dev", uevent->device_name))
-                    return;
-                mkdir_recursive_for_devpath(devpath);
-             }
-             else {
-                 /* This imitates the file system that would be created
-                  * if we were using devfs instead.
-                  * Minors are broken up into groups of 128, starting at "001"
-                  */
-                 int bus_id = uevent->minor / 128 + 1;
-                 int device_id = uevent->minor % 128 + 1;
-                 /* build directories */
-                 make_dir("/dev/bus", 0755);
-                 make_dir("/dev/bus/usb", 0755);
-                 snprintf(devpath, sizeof(devpath), "/dev/bus/usb/%03d", bus_id);
-                 make_dir(devpath, 0755);
-                 snprintf(devpath, sizeof(devpath), "/dev/bus/usb/%03d/%03d", bus_id, device_id);
-             }
-         } else {
-             /* ignore other USB events */
-             return;
-         }
-     } else if (!strncmp(uevent->subsystem, "graphics", 8)) {
-         base = "/dev/graphics/";
-         make_dir(base, 0755);
-     } else if (!strncmp(uevent->subsystem, "drm", 3)) {
-         base = "/dev/dri/";
-         make_dir(base, 0755);
-     } else if (!strncmp(uevent->subsystem, "oncrpc", 6)) {
-         base = "/dev/oncrpc/";
-         make_dir(base, 0755);
-     } else if (!strncmp(uevent->subsystem, "adsp", 4)) {
-         base = "/dev/adsp/";
-         make_dir(base, 0755);
-     } else if (!strncmp(uevent->subsystem, "msm_camera", 10)) {
-         base = "/dev/msm_camera/";
-         make_dir(base, 0755);
-     } else if(!strncmp(uevent->subsystem, "input", 5)) {
-         base = "/dev/input/";
-         make_dir(base, 0755);
-     } else if(!strncmp(uevent->subsystem, "mtd", 3)) {
-         base = "/dev/mtd/";
-         make_dir(base, 0755);
-     } else if(!strncmp(uevent->subsystem, "sound", 5)) {
-         base = "/dev/snd/";
-         make_dir(base, 0755);
-     } else
-         base = "/dev/";
-     auto links = get_character_device_symlinks(uevent);
+                // TODO: Remove std::string
+                devpath = "/dev/" + std::string(uevent->device_name);
+            } else {
+                // This imitates the file system that would be created
+                // if we were using devfs instead.
+                // Minors are broken up into groups of 128, starting at "001"
+                int bus_id = uevent->minor / 128 + 1;
+                int device_id = uevent->minor % 128 + 1;
+                devpath = android::base::StringPrintf("/dev/bus/usb/%03d/%03d", bus_id, device_id);
+            }
+            mkdir_recursive(android::base::Dirname(devpath), 0755);
+        } else {
+            // ignore other USB events
+            return;
+        }
+    } else {
+        devpath = "/dev/" + name;
+    }
 
-     if (!devpath[0])
-         snprintf(devpath, sizeof(devpath), "%s%s", base, name);
+    auto links = get_character_device_symlinks(uevent);
 
-     handle_device(uevent->action, devpath, uevent->path, 0,
-             uevent->major, uevent->minor, links);
+    handle_device(uevent->action, devpath.c_str(), uevent->path, 0, uevent->major, uevent->minor,
+                  links);
 }
 
 static void handle_device_event(struct uevent *uevent)

init/util.cpp

@@ -194,37 +194,18 @@ bool write_file(const std::string& path, const std::string& content) {
     return success;
 }
 
-int mkdir_recursive(const char *pathname, mode_t mode)
-{
-    char buf[128];
-    const char *slash;
-    const char *p = pathname;
-    int width;
-    int ret;
-    struct stat info;
-
-    while ((slash = strchr(p, '/')) != NULL) {
-        width = slash - pathname;
-        p = slash + 1;
-        if (width < 0)
-            break;
-        if (width == 0)
-            continue;
-        if ((unsigned int)width > sizeof(buf) - 1) {
-            LOG(ERROR) << "path too long for mkdir_recursive";
-            return -1;
-        }
-        memcpy(buf, pathname, width);
-        buf[width] = 0;
-        if (stat(buf, &info) != 0) {
-            ret = make_dir(buf, mode);
-            if (ret && errno != EEXIST)
-                return ret;
+int mkdir_recursive(const std::string& path, mode_t mode) {
+    std::string::size_type slash = 0;
+    while ((slash = path.find('/', slash + 1)) != std::string::npos) {
+        auto directory = path.substr(0, slash);
+        struct stat info;
+        if (stat(directory.c_str(), &info) != 0) {
+            auto ret = make_dir(directory.c_str(), mode);
+            if (ret && errno != EEXIST) return ret;
         }
     }
-    ret = make_dir(pathname, mode);
-    if (ret && errno != EEXIST)
-        return ret;
+    auto ret = make_dir(path.c_str(), mode);
+    if (ret && errno != EEXIST) return ret;
     return 0;
 }
 

init/util.h

@@ -60,7 +60,7 @@ std::ostream& operator<<(std::ostream& os, const Timer& t);
 
 unsigned int decode_uid(const char *s);
 
-int mkdir_recursive(const char *pathname, mode_t mode);
+int mkdir_recursive(const std::string& pathname, mode_t mode);
 int wait_for_file(const char *filename, std::chrono::nanoseconds timeout);
 void import_kernel_cmdline(bool in_qemu,
                            const std::function<void(const std::string&, const std::string&, bool)>&);

init/util_test.cpp

@@ -120,3 +120,39 @@ TEST(util, decode_uid) {
   EXPECT_EQ(UINT_MAX, decode_uid("toot"));
   EXPECT_EQ(123U, decode_uid("123"));
 }
+
+TEST(util, is_dir) {
+    TemporaryDir test_dir;
+    EXPECT_TRUE(is_dir(test_dir.path));
+    TemporaryFile tf;
+    EXPECT_FALSE(is_dir(tf.path));
+}
+
+// sehandle is needed for make_dir()
+// TODO: Remove once sehandle is encapsulated
+#include <selinux/label.h>
+selabel_handle* sehandle;
+
+TEST(util, mkdir_recursive) {
+    TemporaryDir test_dir;
+    std::string path = android::base::StringPrintf("%s/three/directories/deep", test_dir.path);
+    EXPECT_EQ(0, mkdir_recursive(path, 0755));
+    std::string path1 = android::base::StringPrintf("%s/three", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+    std::string path2 = android::base::StringPrintf("%s/three/directories", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+    std::string path3 = android::base::StringPrintf("%s/three/directories/deep", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+}
+
+TEST(util, mkdir_recursive_extra_slashes) {
+    TemporaryDir test_dir;
+    std::string path = android::base::StringPrintf("%s/three////directories/deep//", test_dir.path);
+    EXPECT_EQ(0, mkdir_recursive(path, 0755));
+    std::string path1 = android::base::StringPrintf("%s/three", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+    std::string path2 = android::base::StringPrintf("%s/three/directories", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+    std::string path3 = android::base::StringPrintf("%s/three/directories/deep", test_dir.path);
+    EXPECT_TRUE(is_dir(path1.c_str()));
+}

rootdir/ueventd.rc

@@ -1,5 +1,37 @@
 subsystem adf
-	devname uevent_devname
+    devname uevent_devname
+
+subsystem graphics
+    devname uevent_devpath
+    dirname /dev/graphics
+
+subsystem drm
+    devname uevent_devpath
+    dirname /dev/dri
+
+subsystem oncrpc
+    devname uevent_devpath
+    dirname /dev/oncrpc
+
+subsystem adsp
+    devname uevent_devpath
+    dirname /dev/adsp
+
+subsystem msm_camera
+    devname uevent_devpath
+    dirname /dev/msm_camera
+
+subsystem input
+    devname uevent_devpath
+    dirname /dev/input
+
+subsystem mtd
+    devname uevent_devpath
+    dirname /dev/mtd
+
+subsystem sound
+    devname uevent_devpath
+    dirname /dev/snd
 
 # ueventd can only set permissions on device nodes and their associated
 # sysfs attributes, not on arbitrary paths.