From bd97980193a32c4b87c64dd4a041d25c7f841d5e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jan 2022 00:13:28 +0900 Subject: [PATCH 1/4] udevadm: do not remove watch directory See the comment in the code. --- src/udev/udevadm-info.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index 018a52ada8..3535517183 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -313,7 +313,7 @@ static void cleanup_dirs_after_db_cleanup(DIR *dir, DIR *datadir) { } static void cleanup_db(void) { - _cleanup_closedir_ DIR *dir1 = NULL, *dir2 = NULL, *dir3 = NULL, *dir4 = NULL, *dir5 = NULL; + _cleanup_closedir_ DIR *dir1 = NULL, *dir2 = NULL, *dir3 = NULL, *dir4 = NULL; dir1 = opendir("/run/udev/data"); if (dir1) @@ -331,9 +331,8 @@ static void cleanup_db(void) { if (dir4) cleanup_dir(dir4, 0, 2); - dir5 = opendir("/run/udev/watch"); - if (dir5) - cleanup_dir_after_db_cleanup(dir5, dir1); + /* Do not remove /run/udev/watch. It will be handled by udevd well on restart. + * And should not be removed by external program when udevd is running. */ } static int query_device(QueryType query, sd_device* device) { From 9e0bd1d69b006ce4e463003ad38aef22da0c1ac3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jan 2022 00:16:35 +0900 Subject: [PATCH 2/4] udevadm: split assertions Then we can easily find which pointer is NULL. --- src/udev/udevadm-info.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index 3535517183..a825f17c06 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -266,7 +266,8 @@ static void cleanup_dir(DIR *dir, mode_t mask, int depth) { static bool cleanup_dir_after_db_cleanup(DIR *dir, DIR *datadir) { unsigned int kept = 0; - assert(dir && datadir); + assert(dir); + assert(datadir); FOREACH_DIRENT_ALL(dent, dir, break) { struct stat data_stats, link_stats; @@ -291,8 +292,8 @@ static bool cleanup_dir_after_db_cleanup(DIR *dir, DIR *datadir) { } static void cleanup_dirs_after_db_cleanup(DIR *dir, DIR *datadir) { - - assert(dir && datadir); + assert(dir); + assert(datadir); FOREACH_DIRENT_ALL(dent, dir, break) { struct stat stats; From 636ab00182e289489a2c3547c068d7030475e51d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jan 2022 00:35:15 +0900 Subject: [PATCH 3/4] udevadm: simplify the code of removing udev state files --- src/udev/udevadm-info.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index a825f17c06..e30801865e 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -261,34 +261,22 @@ static void cleanup_dir(DIR *dir, mode_t mask, int depth) { * entries for devices in /run/udev/data (such as "b8:16"), and removes * all files except those that haven't been deleted in /run/udev/data * (i.e. they were skipped during db cleanup because of the db_persist flag). - * Returns true if the directory is empty after cleanup. */ -static bool cleanup_dir_after_db_cleanup(DIR *dir, DIR *datadir) { - unsigned int kept = 0; - +static void cleanup_dir_after_db_cleanup(DIR *dir, DIR *datadir) { assert(dir); assert(datadir); FOREACH_DIRENT_ALL(dent, dir, break) { - struct stat data_stats, link_stats; - if (dot_or_dot_dot(dent->d_name)) continue; - if (fstatat(dirfd(dir), dent->d_name, &link_stats, AT_SYMLINK_NOFOLLOW) < 0) { - if (errno != ENOENT) - kept++; + + if (faccessat(dirfd(datadir), dent->d_name, F_OK, AT_SYMLINK_NOFOLLOW) >= 0) + /* The corresponding udev database file still exists. + * Assuming the parsistent flag is set for the database. */ continue; - } - if (fstatat(dirfd(datadir), dent->d_name, &data_stats, 0) < 0) - (void) unlinkat(dirfd(dir), dent->d_name, - S_ISDIR(link_stats.st_mode) ? AT_REMOVEDIR : 0); - else - /* The entry still exists under /run/udev/data */ - kept++; + (void) unlinkat(dirfd(dir), dent->d_name, 0); } - - return kept == 0; } static void cleanup_dirs_after_db_cleanup(DIR *dir, DIR *datadir) { @@ -306,8 +294,10 @@ static void cleanup_dirs_after_db_cleanup(DIR *dir, DIR *datadir) { _cleanup_closedir_ DIR *dir2 = NULL; dir2 = fdopendir(openat(dirfd(dir), dent->d_name, O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC)); - if (dir2 && cleanup_dir_after_db_cleanup(dir2, datadir)) - (void) unlinkat(dirfd(dir), dent->d_name, AT_REMOVEDIR); + if (dir2) + cleanup_dir_after_db_cleanup(dir2, datadir); + + (void) unlinkat(dirfd(dir), dent->d_name, AT_REMOVEDIR); } else (void) unlinkat(dirfd(dir), dent->d_name, 0); } From 4881a0d2d4c34b2c62a9c71be4c9fc826fd4525b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 22 Jan 2022 00:44:12 +0900 Subject: [PATCH 4/4] udevadm: add more assertions --- src/udev/udevadm-info.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/udev/udevadm-info.c b/src/udev/udevadm-info.c index e30801865e..5ae33e101a 100644 --- a/src/udev/udevadm-info.c +++ b/src/udev/udevadm-info.c @@ -47,6 +47,8 @@ static const char *arg_export_prefix = NULL; static usec_t arg_wait_for_initialization_timeout = 0; static bool skip_attribute(const char *name) { + assert(name); + /* Those are either displayed separately or should not be shown at all. */ return STR_IN_SET(name, "uevent", @@ -66,6 +68,9 @@ typedef struct SysAttr { STATIC_DESTRUCTOR_REGISTER(arg_properties, strv_freep); static int sysattr_compare(const SysAttr *a, const SysAttr *b) { + assert(a); + assert(b); + return strcmp(a->name, b->name); } @@ -75,6 +80,8 @@ static int print_all_attributes(sd_device *device, bool is_parent) { size_t n_items = 0; int r; + assert(device); + value = NULL; (void) sd_device_get_devpath(device, &value); printf(" looking at %sdevice '%s':\n", is_parent ? "parent " : "", strempty(value)); @@ -139,6 +146,8 @@ static int print_device_chain(sd_device *device) { sd_device *child, *parent; int r; + assert(device); + printf("\n" "Udevadm info starts with the device specified by the devpath and then\n" "walks up the chain of parent devices. It prints for every device\n" @@ -164,6 +173,8 @@ static int print_record(sd_device *device) { const char *str, *val; int i; + assert(device); + (void) sd_device_get_devpath(device, &str); printf("P: %s\n", str); @@ -190,6 +201,8 @@ static int print_record(sd_device *device) { static int stat_device(const char *name, bool export, const char *prefix) { struct stat statbuf; + assert(name); + if (stat(name, &statbuf) != 0) return -errno; @@ -229,11 +242,11 @@ static int export_devices(void) { } static void cleanup_dir(DIR *dir, mode_t mask, int depth) { + assert(dir); + if (depth <= 0) return; - assert(dir); - FOREACH_DIRENT_ALL(dent, dir, break) { struct stat stats; @@ -389,10 +402,10 @@ static int query_device(QueryType query, sd_device* device) { case QUERY_ALL: return print_record(device); - } - assert_not_reached(); - return 0; + default: + assert_not_reached(); + } } static int help(void) {