ueventd: replace char* with std::string in struct uevent
Bug: 36250207
Test: Boot bullhead
Test: Boot sailfish, observe no boot time regression
Test: init unit tests
Change-Id: Ib82833bea56bdafbe1d7a045126aaa91a8725d98
diff --git a/init/devices.cpp b/init/devices.cpp
index 6cd3564..92eb6cf 100644
--- a/init/devices.cpp
+++ b/init/devices.cpp
@@ -56,9 +56,6 @@
#include "util.h"
#define SYSFS_PREFIX "/sys"
-static const char *firmware_dirs[] = { "/etc/firmware",
- "/vendor/firmware",
- "/firmware/image" };
extern struct selabel_handle *sehandle;
@@ -153,17 +150,18 @@
return perm_path_matches(subsys_path.c_str(), dp);
}
-static void fixup_sys_perms(const char* upath, const char* subsystem) {
+static void fixup_sys_perms(const std::string& upath, const std::string& subsystem) {
// upaths omit the "/sys" that paths in this list
// contain, so we prepend it...
- std::string path = std::string(SYSFS_PREFIX) + upath;
+ std::string path = SYSFS_PREFIX + upath;
listnode* node;
list_for_each(node, &sys_perms) {
perms_* dp = &(node_to_item(node, perm_node, plist))->dp;
- if (match_subsystem(dp, SYSFS_PREFIX "/class/%s/%s", path.c_str(), subsystem)) {
+ if (match_subsystem(dp, SYSFS_PREFIX "/class/%s/%s", path.c_str(), subsystem.c_str())) {
; // matched
- } else if (match_subsystem(dp, SYSFS_PREFIX "/bus/%s/devices/%s", path.c_str(), subsystem)) {
+ } else if (match_subsystem(dp, SYSFS_PREFIX "/bus/%s/devices/%s", path.c_str(),
+ subsystem.c_str())) {
; // matched
} else if (!perm_path_matches(path.c_str(), dp)) {
continue;
@@ -209,7 +207,7 @@
return 0600;
}
-static void make_device(const char* path, const char* /*upath*/, int block, int major, int minor,
+static void make_device(const std::string& path, int block, int major, int minor,
const std::vector<std::string>& links) {
unsigned uid;
unsigned gid;
@@ -217,7 +215,7 @@
dev_t dev;
char *secontext = NULL;
- mode = get_device_perm(path, links, &uid, &gid) | (block ? S_IFBLK : S_IFCHR);
+ mode = get_device_perm(path.c_str(), links, &uid, &gid) | (block ? S_IFBLK : S_IFCHR);
if (sehandle) {
std::vector<const char*> c_links;
@@ -225,7 +223,7 @@
c_links.emplace_back(link.c_str());
}
c_links.emplace_back(nullptr);
- if (selabel_lookup_best_match(sehandle, &secontext, path, &c_links[0], mode)) {
+ if (selabel_lookup_best_match(sehandle, &secontext, path.c_str(), &c_links[0], mode)) {
PLOG(ERROR) << "Device '" << path << "' not created; cannot find SELinux label";
return;
}
@@ -244,10 +242,9 @@
}
/* If the node already exists update its SELinux label to handle cases when
* it was created with the wrong context during coldboot procedure. */
- if (mknod(path, mode, dev) && (errno == EEXIST) && secontext) {
-
+ if (mknod(path.c_str(), mode, dev) && (errno == EEXIST) && secontext) {
char* fcon = nullptr;
- int rc = lgetfilecon(path, &fcon);
+ int rc = lgetfilecon(path.c_str(), &fcon);
if (rc < 0) {
PLOG(ERROR) << "Cannot get SELinux label on '" << path << "' device";
goto out;
@@ -256,13 +253,13 @@
bool different = strcmp(fcon, secontext) != 0;
freecon(fcon);
- if (different && lsetfilecon(path, secontext)) {
+ if (different && lsetfilecon(path.c_str(), secontext)) {
PLOG(ERROR) << "Cannot set '" << secontext << "' SELinux label on '" << path << "' device";
}
}
out:
- chown(path, uid, -1);
+ chown(path.c_str(), uid, -1);
if (setegid(AID_ROOT)) {
PLOG(FATAL) << "setegid(AID_ROOT) failed";
}
@@ -298,8 +295,7 @@
* platform device prefix. If it doesn't start with a platform device, return
* 0.
*/
-static struct platform_node *find_platform_device(const char *path)
-{
+static struct platform_node* find_platform_device(const char* path) {
int path_len = strlen(path);
struct listnode *node;
struct platform_node *bus;
@@ -398,19 +394,11 @@
return true;
}
-static void parse_event(const char *msg, struct uevent *uevent)
-{
- uevent->action = "";
- uevent->path = "";
- uevent->subsystem = "";
- uevent->firmware = "";
+void parse_event(const char* msg, uevent* uevent) {
+ uevent->partition_num = -1;
uevent->major = -1;
uevent->minor = -1;
- uevent->partition_name = NULL;
- uevent->partition_num = -1;
- uevent->device_name = NULL;
-
- /* currently ignoring SEQNUM */
+ // currently ignoring SEQNUM
while(*msg) {
if(!strncmp(msg, "ACTION=", 7)) {
msg += 7;
@@ -441,28 +429,27 @@
uevent->device_name = msg;
}
- /* advance to after the next \0 */
+ // advance to after the next \0
while(*msg++)
;
}
if (LOG_UEVENTS) {
- LOG(INFO) << android::base::StringPrintf("event { '%s', '%s', '%s', '%s', %d, %d }",
- uevent->action, uevent->path, uevent->subsystem,
- uevent->firmware, uevent->major, uevent->minor);
+ LOG(INFO) << "event { '" << uevent->action << "', '" << uevent->path << "', '"
+ << uevent->subsystem << "', '" << uevent->firmware << "', " << uevent->major
+ << ", " << uevent->minor << " }";
}
}
std::vector<std::string> get_character_device_symlinks(uevent* uevent) {
- platform_node* pdev = find_platform_device(uevent->path);
+ platform_node* pdev = find_platform_device(uevent->path.c_str());
if (!pdev) return {};
/* skip "/devices/platform/<driver>" */
- std::string parent = std::string(uevent->path);
- auto parent_start = parent.find('/', pdev->path_len);
+ auto parent_start = uevent->path.find('/', pdev->path_len);
if (parent_start == std::string::npos) return {};
- parent.erase(0, parent_start);
+ std::string parent = uevent->path.substr(parent_start);
if (!android::base::StartsWith(parent, "/usb")) return {};
@@ -488,9 +475,8 @@
auto name_string = parent.substr(start, length);
- // TODO: remove std::string() when uevent->subsystem is an std::string
std::vector<std::string> links;
- links.emplace_back("/dev/usb/" + std::string(uevent->subsystem) + name_string);
+ links.emplace_back("/dev/usb/" + uevent->subsystem + name_string);
mkdir("/dev/usb", 0755);
@@ -519,7 +505,7 @@
struct platform_node* pdev;
std::string type;
- pdev = find_platform_device(uevent->path);
+ pdev = find_platform_device(uevent->path.c_str());
if (pdev) {
device = pdev->name;
type = "platform";
@@ -537,7 +523,7 @@
auto link_path = "/dev/block/" + type + "/" + device;
- if (uevent->partition_name) {
+ if (!uevent->partition_name.empty()) {
std::string partition_name_sanitized(uevent->partition_name);
sanitize_partition_name(&partition_name_sanitized);
if (partition_name_sanitized != uevent->partition_name) {
@@ -551,57 +537,50 @@
links.emplace_back(link_path + "/by-num/p" + std::to_string(uevent->partition_num));
}
- // TODO: remove uevent_path when uevent->path is an std::string
- std::string uevent_path = uevent->path;
- auto last_slash = uevent_path.rfind('/');
- links.emplace_back(link_path + "/" + uevent_path.substr(last_slash + 1));
+ auto last_slash = uevent->path.rfind('/');
+ links.emplace_back(link_path + "/" + uevent->path.substr(last_slash + 1));
return links;
}
-static void make_link_init(const char* oldpath, const char* newpath) {
- const char* slash = strrchr(newpath, '/');
- if (!slash) return;
+static void make_link_init(const std::string& oldpath, const std::string& newpath) {
+ if (mkdir_recursive(dirname(newpath.c_str()), 0755)) {
+ PLOG(ERROR) << "Failed to create directory " << dirname(newpath.c_str());
+ }
- if (mkdir_recursive(dirname(newpath), 0755)) {
- PLOG(ERROR) << "Failed to create directory " << dirname(newpath);
- }
-
- if (symlink(oldpath, newpath) && errno != EEXIST) {
- PLOG(ERROR) << "Failed to symlink " << oldpath << " to " << newpath;
- }
+ if (symlink(oldpath.c_str(), newpath.c_str()) && errno != EEXIST) {
+ PLOG(ERROR) << "Failed to symlink " << oldpath << " to " << newpath;
+ }
}
-static void remove_link(const char* oldpath, const char* newpath) {
- std::string path;
- if (android::base::Readlink(newpath, &path) && path == oldpath) unlink(newpath);
+static void remove_link(const std::string& oldpath, const std::string& newpath) {
+ std::string path;
+ if (android::base::Readlink(newpath, &path) && path == oldpath) unlink(newpath.c_str());
}
-static void handle_device(const char* action, const char* devpath, const char* path, int block,
+static void handle_device(const std::string& action, const std::string& devpath, int block,
int major, int minor, const std::vector<std::string>& links) {
- if(!strcmp(action, "add")) {
- make_device(devpath, path, block, major, minor, links);
+ if (action == "add") {
+ make_device(devpath, block, major, minor, links);
for (const auto& link : links) {
- make_link_init(devpath, link.c_str());
+ make_link_init(devpath, link);
}
}
- if(!strcmp(action, "remove")) {
+ if (action == "remove") {
for (const auto& link : links) {
- remove_link(devpath, link.c_str());
+ remove_link(devpath, link);
}
- unlink(devpath);
+ unlink(devpath.c_str());
}
}
-static void handle_platform_device_event(struct uevent *uevent)
-{
- const char *path = uevent->path;
-
- if (!strcmp(uevent->action, "add"))
- add_platform_device(path);
- else if (!strcmp(uevent->action, "remove"))
- remove_platform_device(path);
+static void handle_platform_device_event(uevent* uevent) {
+ if (uevent->action == "add") {
+ add_platform_device(uevent->path.c_str());
+ } else if (uevent->action == "remove") {
+ remove_platform_device(uevent->path.c_str());
+ }
}
static void handle_block_device_event(uevent* uevent) {
@@ -615,11 +594,11 @@
std::string devpath = base + name;
std::vector<std::string> links;
- if (!strncmp(uevent->path, "/devices/", 9))
+ if (android::base::StartsWith(uevent->path, "/devices")) {
links = get_block_device_symlinks(uevent);
+ }
- handle_device(uevent->action, devpath.c_str(), uevent->path, 1, uevent->major, uevent->minor,
- links);
+ handle_device(uevent->action, devpath, 1, uevent->major, uevent->minor, links);
}
static void handle_generic_device_event(uevent* uevent) {
@@ -627,7 +606,7 @@
if (uevent->major < 0 || uevent->minor < 0) return;
std::string name = android::base::Basename(uevent->path);
- ueventd_subsystem* subsystem = ueventd_subsystem_find_by_name(uevent->subsystem);
+ ueventd_subsystem* subsystem = ueventd_subsystem_find_by_name(uevent->subsystem.c_str());
std::string devpath;
@@ -651,11 +630,10 @@
// 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 (uevent->device_name) {
- // TODO: Remove std::string
- devpath = "/dev/" + std::string(uevent->device_name);
+ } else if (android::base::StartsWith(uevent->subsystem, "usb")) {
+ if (uevent->subsystem == "usb") {
+ if (!uevent->device_name.empty()) {
+ devpath = "/dev/" + uevent->device_name;
} else {
// This imitates the file system that would be created
// if we were using devfs instead.
@@ -675,18 +653,18 @@
auto links = get_character_device_symlinks(uevent);
- handle_device(uevent->action, devpath.c_str(), uevent->path, 0, uevent->major, uevent->minor,
- links);
+ handle_device(uevent->action, devpath, 0, uevent->major, uevent->minor, links);
}
static void handle_device_event(struct uevent *uevent)
{
- if (!strcmp(uevent->action,"add") || !strcmp(uevent->action, "change") || !strcmp(uevent->action, "online"))
+ if (uevent->action == "add" || uevent->action == "change" || uevent->action == "online") {
fixup_sys_perms(uevent->path, uevent->subsystem);
+ }
- if (!strncmp(uevent->subsystem, "block", 5)) {
+ if (uevent->subsystem == "block") {
handle_block_device_event(uevent);
- } else if (!strncmp(uevent->subsystem, "platform", 8)) {
+ } else if (uevent->subsystem == "platform") {
handle_platform_device_event(uevent);
} else {
handle_generic_device_event(uevent);
@@ -719,7 +697,7 @@
LOG(INFO) << "firmware: loading '" << uevent->firmware << "' for '" << uevent->path << "'";
- std::string root = android::base::StringPrintf("/sys%s", uevent->path);
+ std::string root = "/sys" + uevent->path;
std::string loading = root + "/loading";
std::string data = root + "/data";
@@ -735,9 +713,12 @@
return;
}
+ static const char* firmware_dirs[] = {"/etc/firmware/", "/vendor/firmware/",
+ "/firmware/image/"};
+
try_loading_again:
for (size_t i = 0; i < arraysize(firmware_dirs); i++) {
- std::string file = android::base::StringPrintf("%s/%s", firmware_dirs[i], uevent->firmware);
+ std::string file = firmware_dirs[i] + uevent->firmware;
android::base::unique_fd fw_fd(open(file.c_str(), O_RDONLY|O_CLOEXEC));
struct stat sb;
if (fw_fd != -1 && fstat(fw_fd, &sb) != -1) {
@@ -761,8 +742,7 @@
}
static void handle_firmware_event(uevent* uevent) {
- if (strcmp(uevent->subsystem, "firmware")) return;
- if (strcmp(uevent->action, "add")) return;
+ if (uevent->subsystem != "firmware" || uevent->action != "add") return;
// Loading the firmware in a child means we can do that in parallel...
// (We ignore SIGCHLD rather than wait for our children.)
@@ -796,7 +776,7 @@
msg[n] = '\0';
msg[n+1] = '\0';
- struct uevent uevent;
+ uevent uevent;
parse_event(msg, &uevent);
coldboot_action_t act = handle_uevent(&uevent);
if (should_stop_coldboot(act))
diff --git a/init/devices.h b/init/devices.h
index f6183c9..c12ba9d 100644
--- a/init/devices.h
+++ b/init/devices.h
@@ -36,12 +36,12 @@
};
struct uevent {
- const char* action;
- const char* path;
- const char* subsystem;
- const char* firmware;
- const char* partition_name;
- const char* device_name;
+ std::string action;
+ std::string path;
+ std::string subsystem;
+ std::string firmware;
+ std::string partition_name;
+ std::string device_name;
int partition_num;
int major;
int minor;
diff --git a/init/devices_test.cpp b/init/devices_test.cpp
index 5be0d88..b6b4b82 100644
--- a/init/devices_test.cpp
+++ b/init/devices_test.cpp
@@ -127,7 +127,7 @@
const char* platform_device = "/devices/soc.0/f9824900.sdhci";
uevent uevent = {
.path = "/devices/soc.0/f9824900.sdhci/mmc_host/mmc0/mmc0:0001/block/mmcblk0",
- .partition_name = nullptr,
+ .partition_name = "",
.partition_num = -1,
};
std::vector<std::string> expected_result{"/dev/block/platform/soc.0/f9824900.sdhci/mmcblk0"};
@@ -156,7 +156,7 @@
const char* platform_device = "/devices/soc.0/f9824900.sdhci";
uevent uevent = {
.path = "/devices/soc.0/f9824900.sdhci/mmc_host/mmc0/mmc0:0001/block/mmcblk0p1",
- .partition_name = nullptr,
+ .partition_name = "",
.partition_num = 1,
};
std::vector<std::string> expected_result{
@@ -185,9 +185,7 @@
TEST(devices, get_block_device_symlinks_success_pci) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
- .path = "/devices/pci0000:00/0000:00:1f.2/mmcblk0",
- .partition_name = nullptr,
- .partition_num = -1,
+ .path = "/devices/pci0000:00/0000:00:1f.2/mmcblk0", .partition_name = "", .partition_num = -1,
};
std::vector<std::string> expected_result{"/dev/block/pci/pci0000:00/0000:00:1f.2/mmcblk0"};
@@ -197,7 +195,7 @@
TEST(devices, get_block_device_symlinks_pci_bad_format) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
- .path = "/devices/pci//mmcblk0", .partition_name = nullptr, .partition_num = -1,
+ .path = "/devices/pci//mmcblk0", .partition_name = "", .partition_num = -1,
};
std::vector<std::string> expected_result{};
@@ -207,7 +205,7 @@
TEST(devices, get_block_device_symlinks_success_vbd) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
- .path = "/devices/vbd-1234/mmcblk0", .partition_name = nullptr, .partition_num = -1,
+ .path = "/devices/vbd-1234/mmcblk0", .partition_name = "", .partition_num = -1,
};
std::vector<std::string> expected_result{"/dev/block/vbd/1234/mmcblk0"};
@@ -217,7 +215,7 @@
TEST(devices, get_block_device_symlinks_vbd_bad_format) {
const char* platform_device = "/devices/do/not/match";
uevent uevent = {
- .path = "/devices/vbd-/mmcblk0", .partition_name = nullptr, .partition_num = -1,
+ .path = "/devices/vbd-/mmcblk0", .partition_name = "", .partition_num = -1,
};
std::vector<std::string> expected_result{};
@@ -228,7 +226,7 @@
const char* platform_device = "/devices/soc.0/f9824900.sdhci";
uevent uevent = {
.path = "/devices/soc.0/not_the_device/mmc_host/mmc0/mmc0:0001/block/mmcblk0p1",
- .partition_name = nullptr,
+ .partition_name = "",
.partition_num = -1,
};
std::vector<std::string> expected_result;
diff --git a/init/init.cpp b/init/init.cpp
index 6c09c99..6c1c541 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -958,7 +958,7 @@
const std::string syspath = "/sys/block/" + device_name;
device_init(syspath.c_str(), [&](uevent* uevent) -> coldboot_action_t {
- if (uevent->device_name && device_name == uevent->device_name) {
+ if (uevent->device_name == device_name) {
LOG(VERBOSE) << "early_mount: creating dm-verity device : " << dm_device;
return COLDBOOT_STOP;
}
@@ -1055,21 +1055,17 @@
return;
}
device_init(nullptr, [=](uevent* uevent) -> coldboot_action_t {
- if (!strncmp(uevent->subsystem, "firmware", 8)) {
- return COLDBOOT_CONTINUE;
- }
-
// we need platform devices to create symlinks
- if (!strncmp(uevent->subsystem, "platform", 8)) {
+ if (uevent->subsystem == "platform") {
return COLDBOOT_CREATE;
}
// Ignore everything that is not a block device
- if (strncmp(uevent->subsystem, "block", 5)) {
+ if (uevent->subsystem != "block") {
return COLDBOOT_CONTINUE;
}
- if (uevent->partition_name) {
+ if (!uevent->partition_name.empty()) {
// match partition names to create device nodes for partitions
// both partition_names and uevent->partition_name have A/B suffix when A/B is used
auto iter = partition_names->find(uevent->partition_name);