From dcfbde3a43d632ff6e286c77e1081087eca59d8e Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 18:05:27 +0900 Subject: [PATCH 1/6] sd-device: make sd_device_get_*() return -ENOENT if the values are not set --- src/libsystemd/sd-device/device-enumerator.c | 15 ++--------- src/libsystemd/sd-device/device-private.c | 12 +++++++++ src/libsystemd/sd-device/sd-device.c | 28 +++++++++++++------- src/libudev/libudev-device.c | 9 ++++--- 4 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/libsystemd/sd-device/device-enumerator.c b/src/libsystemd/sd-device/device-enumerator.c index 27cc358d68..c7e5bf4431 100644 --- a/src/libsystemd/sd-device/device-enumerator.c +++ b/src/libsystemd/sd-device/device-enumerator.c @@ -478,18 +478,6 @@ static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, continue; } - k = sd_device_get_devnum(device, &devnum); - if (k < 0) { - r = k; - continue; - } - - k = sd_device_get_ifindex(device, &ifindex); - if (k < 0) { - r = k; - continue; - } - k = sd_device_get_is_initialized(device, &initialized); if (k < 0) { r = k; @@ -508,7 +496,8 @@ static int enumerator_scan_dir_and_add_devices(sd_device_enumerator *enumerator, */ if (!enumerator->match_allow_uninitialized && !initialized && - (major(devnum) > 0 || ifindex > 0)) + (sd_device_get_devnum(device, &devnum) >= 0 || + (sd_device_get_ifindex(device, &ifindex) >= 0 && ifindex > 0))) continue; if (!match_parent(enumerator, device)) diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c index 1e41773bba..eb27346786 100644 --- a/src/libsystemd/sd-device/device-private.c +++ b/src/libsystemd/sd-device/device-private.c @@ -277,6 +277,9 @@ int device_get_devnode_mode(sd_device *device, mode_t *mode) { if (r < 0) return r; + if (device->devmode == (mode_t) -1) + return -ENOENT; + *mode = device->devmode; return 0; @@ -292,6 +295,9 @@ int device_get_devnode_uid(sd_device *device, uid_t *uid) { if (r < 0) return r; + if (device->devuid == (uid_t) -1) + return -ENOENT; + *uid = device->devuid; return 0; @@ -327,6 +333,9 @@ int device_get_devnode_gid(sd_device *device, gid_t *gid) { if (r < 0) return r; + if (device->devgid == (gid_t) -1) + return -ENOENT; + *gid = device->devgid; return 0; @@ -723,6 +732,9 @@ int device_get_watch_handle(sd_device *device, int *handle) { if (r < 0) return r; + if (device->watch_handle < 0) + return -ENOENT; + *handle = device->watch_handle; return 0; diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index b020e0d55a..1eb71e17db 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -38,6 +38,10 @@ int device_new_aux(sd_device **ret) { *device = (sd_device) { .n_ref = 1, .watch_handle = -1, + .ifindex = -1, + .devmode = (mode_t) -1, + .devuid = (uid_t) -1, + .devgid = (gid_t) -1, }; *ret = device; @@ -575,6 +579,9 @@ _public_ int sd_device_get_ifindex(sd_device *device, int *ifindex) { if (r < 0) return r; + if (device->ifindex < 0) + return -ENOENT; + *ifindex = device->ifindex; return 0; @@ -839,6 +846,9 @@ _public_ int sd_device_get_devtype(sd_device *device, const char **devtype) { if (r < 0) return r; + if (!device->devtype) + return -ENOENT; + *devtype = device->devtype; return 0; @@ -886,6 +896,9 @@ _public_ int sd_device_get_devnum(sd_device *device, dev_t *devnum) { if (r < 0) return r; + if (major(device->devnum) <= 0) + return -ENOENT; + *devnum = device->devnum; return 0; @@ -1053,6 +1066,9 @@ _public_ int sd_device_get_sysnum(sd_device *device, const char **ret) { return r; } + if (!device->sysnum) + return -ENOENT; + *ret = device->sysnum; return 0; @@ -1216,15 +1232,7 @@ int device_get_id_filename(sd_device *device, const char **ret) { if (r < 0) return r; - r = sd_device_get_devnum(device, &devnum); - if (r < 0) - return r; - - r = sd_device_get_ifindex(device, &ifindex); - if (r < 0) - return r; - - if (major(devnum) > 0) { + if (sd_device_get_devnum(device, &devnum) >= 0) { assert(subsystem); /* use dev_t — b259:131072, c254:0 */ @@ -1233,7 +1241,7 @@ int device_get_id_filename(sd_device *device, const char **ret) { major(devnum), minor(devnum)); if (r < 0) return -ENOMEM; - } else if (ifindex > 0) { + } else if (sd_device_get_ifindex(device, &ifindex) >= 0 && ifindex > 0) { /* use netdev ifindex — n3 */ r = asprintf(&id, "n%u", ifindex); if (r < 0) diff --git a/src/libudev/libudev-device.c b/src/libudev/libudev-device.c index 2cd5b48fea..1c3d67a8f5 100644 --- a/src/libudev/libudev-device.c +++ b/src/libudev/libudev-device.c @@ -85,7 +85,8 @@ _public_ dev_t udev_device_get_devnum(struct udev_device *udev_device) { r = sd_device_get_devnum(udev_device->device, &devnum); if (r < 0) { - errno = -r; + if (r != -ENOENT) + errno = -r; return makedev(0, 0); } @@ -131,7 +132,8 @@ _public_ const char *udev_device_get_devtype(struct udev_device *udev_device) { r = sd_device_get_devtype(udev_device->device, &devtype); if (r < 0) { - errno = -r; + if (r != -ENOENT) + errno = -r; return NULL; } @@ -582,7 +584,8 @@ _public_ const char *udev_device_get_sysnum(struct udev_device *udev_device) { r = sd_device_get_sysnum(udev_device->device, &sysnum); if (r < 0) { - errno = -r; + if (r != -ENOENT) + errno = -r; return NULL; } From 403660c508153a65ac3738b1f0884c74feb4acf6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 23:03:22 +0900 Subject: [PATCH 2/6] tree-wide: use streq() instead of streq_ptr() --- src/backlight/backlight.c | 4 ++-- src/login/logind-core.c | 12 ++++++------ src/login/logind-session-device.c | 4 ++-- src/login/logind.c | 2 +- src/rfkill/rfkill.c | 2 +- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 5579d8a8bf..fb8e3742e8 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -88,7 +88,7 @@ static int same_device(sd_device *a, sd_device *b) { if (r < 0) return r; - if (!streq_ptr(a_val, b_val)) + if (!streq(a_val, b_val)) return false; r = sd_device_get_sysname(a, &a_val); @@ -99,7 +99,7 @@ static int same_device(sd_device *a, sd_device *b) { if (r < 0) return r; - return streq_ptr(a_val, b_val); + return streq(a_val, b_val); } static int validate_device(sd_device *device) { diff --git a/src/login/logind-core.c b/src/login/logind-core.c index dac240d294..d2351b9340 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -218,14 +218,14 @@ int manager_add_button(Manager *m, const char *name, Button **_button) { } int manager_process_seat_device(Manager *m, sd_device *d) { - const char *action = NULL; + const char *action; Device *device; int r; assert(m); - (void) sd_device_get_property_value(d, "ACTION", &action); - if (streq_ptr(action, "remove")) { + if (sd_device_get_property_value(d, "ACTION", &action) >= 0 && + streq(action, "remove")) { const char *syspath; r = sd_device_get_syspath(d, &syspath); @@ -285,7 +285,7 @@ int manager_process_seat_device(Manager *m, sd_device *d) { } int manager_process_button_device(Manager *m, sd_device *d) { - const char *action = NULL, *sysname; + const char *action, *sysname; Button *b; int r; @@ -295,8 +295,8 @@ int manager_process_button_device(Manager *m, sd_device *d) { if (r < 0) return r; - (void) sd_device_get_property_value(d, "ACTION", &action); - if (streq_ptr(action, "remove")) { + if (sd_device_get_property_value(d, "ACTION", &action) >= 0 && + streq(action, "remove")) { b = hashmap_get(m->buttons, sysname); if (!b) diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index fbba792ed6..0b798cf0a2 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -255,10 +255,10 @@ static DeviceType detect_device_type(sd_device *dev) { sd_device_get_subsystem(dev, &subsystem) < 0) return type; - if (streq_ptr(subsystem, "drm")) { + if (streq(subsystem, "drm")) { if (startswith(sysname, "card")) type = DEVICE_TYPE_DRM; - } else if (streq_ptr(subsystem, "input")) { + } else if (streq(subsystem, "input")) { if (startswith(sysname, "event")) type = DEVICE_TYPE_EVDEV; } diff --git a/src/login/logind.c b/src/login/logind.c index b9ad1007ae..e90c8575dc 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -580,7 +580,7 @@ static int manager_dispatch_vcsa_udev(sd_event_source *s, int fd, uint32_t reven if (sd_device_get_sysname(d, &name) >= 0 && startswith(name, "vcsa") && sd_device_get_property_value(d, "ACTION", &action) >= 0 && - streq_ptr(action, "remove")) + streq(action, "remove")) seat_preallocate_vts(m->seat0); return 0; diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c index 36b82a4c92..7939c2d2fe 100644 --- a/src/rfkill/rfkill.c +++ b/src/rfkill/rfkill.c @@ -149,7 +149,7 @@ static int wait_for_initialized( if (r < 0) continue; - if (sd_device_get_sysname(t, &name) >= 0 && streq_ptr(name, sysname)) { + if (sd_device_get_sysname(t, &name) >= 0 && streq(name, sysname)) { *ret = TAKE_PTR(t); return 0; } From 8090b41ed534ad00433ef56445f2a61e121baddd Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 23:07:18 +0900 Subject: [PATCH 3/6] gpt-auto-generator: do not assign '*ret' on error --- src/gpt-auto-generator/gpt-auto-generator.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index a088df811f..6b75221426 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -462,34 +462,34 @@ static int open_parent(dev_t devnum, int *ret) { r = sd_device_get_syspath(d, &name); if (r < 0) { log_debug_errno(r, "Device %u:%u does not have a name, ignoring: %m", major(devnum), minor(devnum)); - goto not_found; + return 0; } } r = sd_device_get_parent(d, &parent); if (r < 0) { log_debug_errno(r, "%s: not a partitioned device, ignoring: %m", name); - goto not_found; + return 0; } /* Does it have a devtype? */ r = sd_device_get_devtype(parent, &devtype); if (r < 0) { log_debug_errno(r, "%s: parent doesn't have a device type, ignoring: %m", name); - goto not_found; + return 0; } /* Is this a disk or a partition? We only care for disks... */ if (!streq(devtype, "disk")) { log_debug("%s: parent isn't a raw disk, ignoring.", name); - goto not_found; + return 0; } /* Does it have a device node? */ r = sd_device_get_devname(parent, &node); if (r < 0) { log_debug_errno(r, "%s: parent device does not have device node, ignoring: %m", name); - goto not_found; + return 0; } log_debug("%s: root device %s.", name, node); @@ -497,7 +497,7 @@ static int open_parent(dev_t devnum, int *ret) { r = sd_device_get_devnum(parent, &pn); if (r < 0) { log_debug_errno(r, "%s: parent device is not a proper block device, ignoring: %m", name); - goto not_found; + return 0; } fd = open(node, O_RDONLY|O_CLOEXEC|O_NOCTTY); @@ -506,14 +506,9 @@ static int open_parent(dev_t devnum, int *ret) { *ret = fd; return 1; - -not_found: - *ret = -1; - return 0; } static int enumerate_partitions(dev_t devnum) { - _cleanup_close_ int fd = -1; _cleanup_(dissected_image_unrefp) DissectedImage *m = NULL; int r, k; From 8a80712bcd137b78f5edb7b1bf3d210b8df9323f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 23:09:54 +0900 Subject: [PATCH 4/6] logind-acl: replace strdup()+set_consume() by set_put_strdup() --- src/login/logind-acl.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/login/logind-acl.c b/src/login/logind-acl.c index da197d039c..5453ebb0a1 100644 --- a/src/login/logind-acl.c +++ b/src/login/logind-acl.c @@ -206,12 +206,8 @@ int devnode_acl_all(const char *seat, if (sd_device_get_devname(d, &node) < 0) continue; - n = strdup(node); - if (!n) - return -ENOMEM; - - log_debug("Found udev node %s for seat %s", n, seat); - r = set_consume(nodes, n); + log_debug("Found udev node %s for seat %s", node, seat); + r = set_put_strdup(nodes, node); if (r < 0) return r; } From 2c740afd1613a7ab5e70e39251767a3429c543cc Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 23:12:47 +0900 Subject: [PATCH 5/6] tree-wide: do not assign unused return values --- src/cryptsetup/cryptsetup.c | 16 +++++----------- src/fsck/fsck.c | 3 +-- src/network/networkctl.c | 19 +++++++------------ 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index a622db849b..b590d70676 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -271,7 +271,6 @@ static int parse_options(const char *options) { } static char* disk_description(const char *path) { - static const char name_fields[] = "ID_PART_ENTRY_NAME\0" "DM_NAME\0" @@ -279,9 +278,8 @@ static char* disk_description(const char *path) { "ID_MODEL\0"; _cleanup_(sd_device_unrefp) sd_device *device = NULL; + const char *i, *name; struct stat st; - const char *i; - int r; assert(path); @@ -291,17 +289,13 @@ static char* disk_description(const char *path) { if (!S_ISBLK(st.st_mode)) return NULL; - r = sd_device_new_from_devnum(&device, 'b', st.st_rdev); - if (r < 0) + if (sd_device_new_from_devnum(&device, 'b', st.st_rdev) < 0) return NULL; - NULSTR_FOREACH(i, name_fields) { - const char *name; - - r = sd_device_get_property_value(device, i, &name); - if (r >= 0 && !isempty(name)) + NULSTR_FOREACH(i, name_fields) + if (sd_device_get_property_value(device, i, &name) >= 0 && + !isempty(name)) return strdup(name); - } return NULL; } diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index 50f838a421..132714fddd 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -357,8 +357,7 @@ int main(int argc, char *argv[]) { root_directory = true; } - r = sd_device_get_property_value(dev, "ID_FS_TYPE", &type); - if (r >= 0) { + if (sd_device_get_property_value(dev, "ID_FS_TYPE", &type) >= 0) { r = fsck_exists(type); if (r < 0) log_warning_errno(r, "Couldn't detect if fsck.%s may be used for %s, proceeding: %m", type, device); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index aaf1a14168..c4a8fc8bc4 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -39,16 +39,13 @@ static bool arg_legend = true; static bool arg_all = false; static char *link_get_type_string(unsigned short iftype, sd_device *d) { - const char *t; + const char *t, *devtype; char *p; - if (d) { - const char *devtype = NULL; - - (void) sd_device_get_devtype(d, &devtype); - if (!isempty(devtype)) - return strdup(devtype); - } + if (d && + sd_device_get_devtype(d, &devtype) >= 0 && + !isempty(devtype)) + return strdup(devtype); t = arphrd_to_name(iftype); if (!t) @@ -768,12 +765,10 @@ static int link_status_one( (void) sd_device_get_property_value(d, "ID_NET_DRIVER", &driver); (void) sd_device_get_property_value(d, "ID_PATH", &path); - r = sd_device_get_property_value(d, "ID_VENDOR_FROM_DATABASE", &vendor); - if (r < 0) + if (sd_device_get_property_value(d, "ID_VENDOR_FROM_DATABASE", &vendor) < 0) (void) sd_device_get_property_value(d, "ID_VENDOR", &vendor); - r = sd_device_get_property_value(d, "ID_MODEL_FROM_DATABASE", &model); - if (r < 0) + if (sd_device_get_property_value(d, "ID_MODEL_FROM_DATABASE", &model) < 0) (void) sd_device_get_property_value(d, "ID_MODEL", &model); } From c679e12af1f2985c4ab4346db8e53ca14f4cc626 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 1 Sep 2018 23:13:54 +0900 Subject: [PATCH 6/6] tree-wide: drop unnecessary initializations --- src/login/logind-core.c | 2 +- src/login/logind-session-device.c | 11 ++++++----- src/login/sysfs-show.c | 10 +++++----- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/login/logind-core.c b/src/login/logind-core.c index d2351b9340..32e1c3435a 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -305,7 +305,7 @@ int manager_process_button_device(Manager *m, sd_device *d) { button_free(b); } else { - const char *sn = NULL; + const char *sn; r = manager_add_button(m, sysname, &b); if (r < 0) diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c index 0b798cf0a2..e1e3e8b498 100644 --- a/src/login/logind-session-device.c +++ b/src/login/logind-session-device.c @@ -268,17 +268,18 @@ static DeviceType detect_device_type(sd_device *dev) { static int session_device_verify(SessionDevice *sd) { _cleanup_(sd_device_unrefp) sd_device *p = NULL; + const char *sp, *node; sd_device *dev; - const char *sp = NULL, *node; int r; - if (sd_device_new_from_devnum(&p, 'c', sd->dev) < 0) - return -ENODEV; + r = sd_device_new_from_devnum(&p, 'c', sd->dev); + if (r < 0) + return r; dev = p; - (void) sd_device_get_syspath(dev, &sp); - if (sd_device_get_devname(dev, &node) < 0) + if (sd_device_get_syspath(dev, &sp) < 0 || + sd_device_get_devname(dev, &node) < 0) return -EINVAL; /* detect device type so we can find the correct sysfs parent */ diff --git a/src/login/sysfs-show.c b/src/login/sysfs-show.c index 758f9c2c1d..192c95107b 100644 --- a/src/login/sysfs-show.c +++ b/src/login/sysfs-show.c @@ -40,7 +40,7 @@ static int show_sysfs_one( max_width = n_columns; while (*i_dev < n_dev) { - const char *sysfs, *sn, *name = NULL, *subsystem = NULL, *sysname = NULL; + const char *sysfs, *sn, *name = NULL, *subsystem, *sysname; _cleanup_free_ char *k = NULL, *l = NULL; size_t lookahead; bool is_master; @@ -53,7 +53,10 @@ static int show_sysfs_one( sn = "seat0"; /* Explicitly also check for tag 'seat' here */ - if (!streq(seat, sn) || sd_device_has_tag(dev_list[*i_dev], "seat") <= 0) { + if (!streq(seat, sn) || + sd_device_has_tag(dev_list[*i_dev], "seat") <= 0 || + sd_device_get_subsystem(dev_list[*i_dev], &subsystem) < 0 || + sd_device_get_sysname(dev_list[*i_dev], &sysname) < 0) { (*i_dev)++; continue; } @@ -63,9 +66,6 @@ static int show_sysfs_one( if (sd_device_get_sysattr_value(dev_list[*i_dev], "name", &name) < 0) (void) sd_device_get_sysattr_value(dev_list[*i_dev], "id", &name); - (void) sd_device_get_subsystem(dev_list[*i_dev], &subsystem); - (void) sd_device_get_sysname(dev_list[*i_dev], &sysname); - /* Look if there's more coming after this */ for (lookahead = *i_dev + 1; lookahead < n_dev; lookahead++) { const char *lookahead_sysfs;