From b4dd881ffd2c2b50b5ffd323264b3c0bfd7bffe7 Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Fri, 23 Jun 2017 12:43:48 -0700 Subject: [PATCH] ueventd: remove character device symlinks (/dev/usb/*) While refactoring ueventd, I was looking for code the character device symlinks (/dev/usb/*) that ueventd creates, such that I could test it on a real device. I found none in our tree, and history showing Xoom, which was last supported years ago, was the last user. If this code is in fact obsolete, let's remove it. Test: boot bullhead Test: init unit tests Change-Id: I601f7165eb06d36b31b6dcf69ee9e0a449d81a96 --- init/devices.cpp | 90 ++++++--------------------------- init/devices.h | 8 +-- init/devices_test.cpp | 115 ++++-------------------------------------- 3 files changed, 29 insertions(+), 184 deletions(-) diff --git a/init/devices.cpp b/init/devices.cpp index 215d2eaa4..13cf991ca 100644 --- a/init/devices.cpp +++ b/init/devices.cpp @@ -219,7 +219,7 @@ std::tuple DeviceHandler::GetDevicePermissions( return {0600, 0, 0}; } -void DeviceHandler::MakeDevice(const std::string& path, int block, int major, int minor, +void DeviceHandler::MakeDevice(const std::string& path, bool block, int major, int minor, const std::vector& links) const { auto[mode, uid, gid] = GetDevicePermissions(path, links); mode |= (block ? S_IFBLK : S_IFCHR); @@ -279,45 +279,6 @@ out: } } -std::vector DeviceHandler::GetCharacterDeviceSymlinks(const Uevent& uevent) const { - std::string parent_device; - if (!FindPlatformDevice(uevent.path, &parent_device)) return {}; - - // skip path to the parent driver - std::string path = uevent.path.substr(parent_device.length()); - - if (!StartsWith(path, "/usb")) return {}; - - // skip root hub name and device. use device interface - // skip 3 slashes, including the first / by starting the search at the 1st character, not 0th. - // then extract what comes between the 3rd and 4th slash - // e.g. "/usb/usb_device/name/tty2-1:1.0" -> "name" - - std::string::size_type start = 0; - start = path.find('/', start + 1); - if (start == std::string::npos) return {}; - - start = path.find('/', start + 1); - if (start == std::string::npos) return {}; - - auto end = path.find('/', start + 1); - if (end == std::string::npos) return {}; - - start++; // Skip the first '/' - - auto length = end - start; - if (length == 0) return {}; - - auto name_string = path.substr(start, length); - - std::vector links; - links.emplace_back("/dev/usb/" + uevent.subsystem + name_string); - - mkdir("/dev/usb", 0755); - - return links; -} - // replaces any unacceptable characters with '_', the // length of the resulting string is equal to the input string void SanitizePartitionName(std::string* string) { @@ -385,7 +346,7 @@ std::vector DeviceHandler::GetBlockDeviceSymlinks(const Uevent& uev return links; } -void DeviceHandler::HandleDevice(const std::string& action, const std::string& devpath, int block, +void DeviceHandler::HandleDevice(const std::string& action, const std::string& devpath, bool block, int major, int minor, const std::vector& links) const { if (action == "add") { MakeDevice(devpath, block, major, minor, links); @@ -411,31 +372,26 @@ void DeviceHandler::HandleDevice(const std::string& action, const std::string& d } } -void DeviceHandler::HandleBlockDeviceEvent(const Uevent& uevent) const { - // if it's not a /dev device, nothing to do - if (uevent.major < 0 || uevent.minor < 0) return; - - const char* base = "/dev/block/"; - make_dir(base, 0755, sehandle_); - - std::string name = Basename(uevent.path); - std::string devpath = base + name; - - std::vector links; - if (StartsWith(uevent.path, "/devices")) { - links = GetBlockDeviceSymlinks(uevent); +void DeviceHandler::HandleDeviceEvent(const Uevent& uevent) { + if (uevent.action == "add" || uevent.action == "change" || uevent.action == "online") { + FixupSysPermissions(uevent.path, uevent.subsystem); } - HandleDevice(uevent.action, devpath, 1, uevent.major, uevent.minor, links); -} - -void DeviceHandler::HandleGenericDeviceEvent(const Uevent& uevent) const { // if it's not a /dev device, nothing to do if (uevent.major < 0 || uevent.minor < 0) return; std::string devpath; + std::vector links; + bool block = false; - if (StartsWith(uevent.subsystem, "usb")) { + if (uevent.subsystem == "block") { + block = true; + devpath = "/dev/block/" + Basename(uevent.path); + + if (StartsWith(uevent.path, "/devices")) { + links = GetBlockDeviceSymlinks(uevent); + } + } else if (StartsWith(uevent.subsystem, "usb")) { if (uevent.subsystem == "usb") { if (!uevent.device_name.empty()) { devpath = "/dev/" + uevent.device_name; @@ -461,21 +417,7 @@ void DeviceHandler::HandleGenericDeviceEvent(const Uevent& uevent) const { mkdir_recursive(Dirname(devpath), 0755, sehandle_); - auto links = GetCharacterDeviceSymlinks(uevent); - - HandleDevice(uevent.action, devpath, 0, uevent.major, uevent.minor, links); -} - -void DeviceHandler::HandleDeviceEvent(const Uevent& uevent) { - if (uevent.action == "add" || uevent.action == "change" || uevent.action == "online") { - FixupSysPermissions(uevent.path, uevent.subsystem); - } - - if (uevent.subsystem == "block") { - HandleBlockDeviceEvent(uevent); - } else { - HandleGenericDeviceEvent(uevent); - } + HandleDevice(uevent.action, devpath, block, uevent.major, uevent.minor, links); } DeviceHandler::DeviceHandler(std::vector dev_permissions, diff --git a/init/devices.h b/init/devices.h index 5105ad757..c64f5fb97 100644 --- a/init/devices.h +++ b/init/devices.h @@ -115,16 +115,12 @@ class DeviceHandler { bool FindPlatformDevice(std::string path, std::string* platform_device_path) const; std::tuple GetDevicePermissions( const std::string& path, const std::vector& links) const; - void MakeDevice(const std::string& path, int block, int major, int minor, + void MakeDevice(const std::string& path, bool block, int major, int minor, const std::vector& links) const; - std::vector GetCharacterDeviceSymlinks(const Uevent& uevent) const; - void HandleDevice(const std::string& action, const std::string& devpath, int block, int major, + void HandleDevice(const std::string& action, const std::string& devpath, bool block, int major, int minor, const std::vector& links) const; void FixupSysPermissions(const std::string& upath, const std::string& subsystem) const; - void HandleBlockDeviceEvent(const Uevent& uevent) const; - void HandleGenericDeviceEvent(const Uevent& uevent) const; - std::vector dev_permissions_; std::vector sysfs_permissions_; std::vector subsystems_; diff --git a/init/devices_test.cpp b/init/devices_test.cpp index 57d8e0f5b..ac4ab9b30 100644 --- a/init/devices_test.cpp +++ b/init/devices_test.cpp @@ -30,7 +30,7 @@ namespace init { class DeviceHandlerTester { public: void TestGetSymlinks(const std::string& platform_device, const Uevent& uevent, - const std::vector expected_links, bool block) { + const std::vector expected_links) { TemporaryDir fake_sys_root; device_handler_.sysfs_mount_point_ = fake_sys_root.path; @@ -44,11 +44,7 @@ class DeviceHandlerTester { mkdir_recursive(android::base::Dirname(fake_sys_root.path + uevent.path), 0777, nullptr); std::vector result; - if (block) { - result = device_handler_.GetBlockDeviceSymlinks(uevent); - } else { - result = device_handler_.GetCharacterDeviceSymlinks(uevent); - } + result = device_handler_.GetBlockDeviceSymlinks(uevent); auto expected_size = expected_links.size(); ASSERT_EQ(expected_size, result.size()); @@ -64,95 +60,6 @@ class DeviceHandlerTester { DeviceHandler device_handler_; }; -TEST(device_handler, get_character_device_symlinks_success) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/usb/usb_device/name/tty2-1:1.0", - .subsystem = "tty", - }; - std::vector expected_result{"/dev/usb/ttyname"}; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_pdev_match) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/device/name/tty2-1:1.0", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_nothing_after_platform_device) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_usb_found) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/bad/bad/", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_roothub) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/usb/", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_usb_device) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/usb/usb_device/", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_final_slash) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/usb/usb_device/name", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - -TEST(device_handler, get_character_device_symlinks_no_final_name) { - const char* platform_device = "/devices/platform/some_device_name"; - Uevent uevent = { - .path = "/devices/platform/some_device_name/usb/usb_device//", .subsystem = "tty", - }; - std::vector expected_result; - - DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, false); -} - TEST(device_handler, get_block_device_symlinks_success_platform) { // These are actual paths from bullhead const char* platform_device = "/devices/soc.0/f9824900.sdhci"; @@ -164,7 +71,7 @@ TEST(device_handler, get_block_device_symlinks_success_platform) { std::vector expected_result{"/dev/block/platform/soc.0/f9824900.sdhci/mmcblk0"}; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_success_platform_with_partition) { @@ -182,7 +89,7 @@ TEST(device_handler, get_block_device_symlinks_success_platform_with_partition) }; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_success_platform_with_partition_only_num) { @@ -198,7 +105,7 @@ TEST(device_handler, get_block_device_symlinks_success_platform_with_partition_o }; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_success_platform_with_partition_only_name) { @@ -214,7 +121,7 @@ TEST(device_handler, get_block_device_symlinks_success_platform_with_partition_o }; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_success_pci) { @@ -225,7 +132,7 @@ TEST(device_handler, get_block_device_symlinks_success_pci) { std::vector expected_result{"/dev/block/pci/pci0000:00/0000:00:1f.2/mmcblk0"}; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_pci_bad_format) { @@ -236,7 +143,7 @@ TEST(device_handler, get_block_device_symlinks_pci_bad_format) { std::vector expected_result{}; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_success_vbd) { @@ -247,7 +154,7 @@ TEST(device_handler, get_block_device_symlinks_success_vbd) { std::vector expected_result{"/dev/block/vbd/1234/mmcblk0"}; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_vbd_bad_format) { @@ -258,7 +165,7 @@ TEST(device_handler, get_block_device_symlinks_vbd_bad_format) { std::vector expected_result{}; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, get_block_device_symlinks_no_matches) { @@ -271,7 +178,7 @@ TEST(device_handler, get_block_device_symlinks_no_matches) { std::vector expected_result; DeviceHandlerTester device_handler_tester_; - device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result, true); + device_handler_tester_.TestGetSymlinks(platform_device, uevent, expected_result); } TEST(device_handler, sanitize_null) {