From 023e75df4c2904e493c4c8ff62df9fa99709d408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:09:11 +0200 Subject: [PATCH 1/7] sd-device: header needs an include because it uses sd_device type --- src/libsystemd/sd-device/device-util.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 122620953a..88c9d9881e 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -1,6 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +#include "sd-device.h" + #define FOREACH_DEVICE_PROPERTY(device, key, value) \ for (key = sd_device_get_property_first(device, &(value)); \ key; \ From 0246f42980ed87dfca79fd4a8ec67a81d824e427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:11:30 +0200 Subject: [PATCH 2/7] test-device-util: let's verify that we return proper error from log_device_* --- src/libsystemd/meson.build | 2 ++ src/libsystemd/sd-device/test-device-util.c | 30 +++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/libsystemd/sd-device/test-device-util.c diff --git a/src/libsystemd/meson.build b/src/libsystemd/meson.build index ad50815a7b..3def5655cc 100644 --- a/src/libsystemd/meson.build +++ b/src/libsystemd/meson.build @@ -307,6 +307,8 @@ tests += [ [['src/libsystemd/sd-device/test-sd-device.c']], + [['src/libsystemd/sd-device/test-device-util.c']], + [['src/libsystemd/sd-device/test-sd-device-monitor.c']], ] diff --git a/src/libsystemd/sd-device/test-device-util.c b/src/libsystemd/sd-device/test-device-util.c new file mode 100644 index 0000000000..93fc105d98 --- /dev/null +++ b/src/libsystemd/sd-device/test-device-util.c @@ -0,0 +1,30 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "device-util.h" +#include "tests.h" + +static void test_log_device_full(void) { + int r; + + log_info("/* %s */", __func__); + + for (int level = LOG_ERR; level <= LOG_DEBUG; level++) { + log_device_full(NULL, level, "test level=%d: %m", level); + + r = log_device_full_errno(NULL, level, EUCLEAN, "test level=%d errno=EUCLEAN: %m", level); + assert_se(r == -EUCLEAN); + + r = log_device_full_errno(NULL, level, 0, "test level=%d errno=0: %m", level); + assert_se(r == 0); + + r = log_device_full_errno(NULL, level, SYNTHETIC_ERRNO(ENODATA), "test level=%d errno=S(ENODATA): %m", level); + assert_se(r == -ENODATA); + } +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_INFO); + + test_log_device_full(); + return 0; +} From c03916164d08f5eb8e24daa21a9e29871bdfe2ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:19:07 +0200 Subject: [PATCH 3/7] backlight: refactor get_max_brightness() to appease gcc The old code was just fine, but gcc doesn't understand that max_brightness is initialized. Let's rework it a bit to move some logic to the main function. Now get_max_brightness() just retrieves and parses the attribute, and the main function decides what to do with it. --- src/backlight/backlight.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/backlight/backlight.c b/src/backlight/backlight.c index 86927be62e..7c0970a60c 100644 --- a/src/backlight/backlight.c +++ b/src/backlight/backlight.c @@ -226,26 +226,20 @@ static int validate_device(sd_device *device) { } static int get_max_brightness(sd_device *device, unsigned *ret) { - const char *max_brightness_str; - unsigned max_brightness; + const char *s; int r; assert(device); assert(ret); - r = sd_device_get_sysattr_value(device, "max_brightness", &max_brightness_str); + r = sd_device_get_sysattr_value(device, "max_brightness", &s); if (r < 0) return log_device_warning_errno(device, r, "Failed to read 'max_brightness' attribute: %m"); - r = safe_atou(max_brightness_str, &max_brightness); + r = safe_atou(s, ret); if (r < 0) - return log_device_warning_errno(device, r, "Failed to parse 'max_brightness' \"%s\": %m", max_brightness_str); + return log_device_warning_errno(device, r, "Failed to parse 'max_brightness' \"%s\": %m", s); - if (max_brightness <= 0) - return log_device_warning_errno(device, SYNTHETIC_ERRNO(EINVAL), "Maximum brightness is 0, ignoring device."); - - log_device_debug(device, "Maximum brightness is %u", max_brightness); - *ret = max_brightness; return 0; } @@ -409,6 +403,13 @@ static int run(int argc, char *argv[]) { if (get_max_brightness(device, &max_brightness) < 0) return 0; + if (max_brightness == 0) { + log_device_warning(device, "Maximum brightness is 0, ignoring device."); + return 0; + } + + log_device_debug(device, "Maximum brightness is %u", max_brightness); + escaped_ss = cescape(ss); if (!escaped_ss) return log_oom(); From 04ab97a829b013dab1b4ba7d8fb66a9de86c08c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:22:50 +0200 Subject: [PATCH 4/7] sd-netlink: drop unnecessary forward declaration --- src/libsystemd/sd-netlink/generic-netlink.c | 23 ++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/libsystemd/sd-netlink/generic-netlink.c b/src/libsystemd/sd-netlink/generic-netlink.c index cd5a0104a6..6760e0d348 100644 --- a/src/libsystemd/sd-netlink/generic-netlink.c +++ b/src/libsystemd/sd-netlink/generic-netlink.c @@ -26,7 +26,6 @@ static const genl_family genl_families[] = { int sd_genl_socket_open(sd_netlink **ret) { return netlink_open_family(ret, NETLINK_GENERIC); } -static int lookup_id(sd_netlink *nl, sd_genl_family_t family, uint16_t *id); static int genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint16_t nlmsg_type, uint8_t cmd, sd_netlink_message **ret) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *m = NULL; @@ -73,17 +72,6 @@ static int genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint16_t nl return 0; } -int sd_genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint8_t cmd, sd_netlink_message **ret) { - uint16_t id; - int r; - - r = lookup_id(nl, family, &id); - if (r < 0) - return r; - - return genl_message_new(nl, family, id, cmd, ret); -} - static int lookup_id(sd_netlink *nl, sd_genl_family_t family, uint16_t *id) { _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL, *reply = NULL; uint16_t u; @@ -129,6 +117,17 @@ static int lookup_id(sd_netlink *nl, sd_genl_family_t family, uint16_t *id) { return 0; } +int sd_genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint8_t cmd, sd_netlink_message **ret) { + uint16_t id; + int r; + + r = lookup_id(nl, family, &id); + if (r < 0) + return r; + + return genl_message_new(nl, family, id, cmd, ret); +} + int nlmsg_type_to_genl_family(const sd_netlink *nl, uint16_t type, sd_genl_family_t *ret) { void *p; From 1f8fb21c42a7b05a0775d50bc9b6680ddaa83e16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:53:42 +0200 Subject: [PATCH 5/7] shared/dissect-image: silence gcc warning --- src/shared/dissect-image.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 70739412a2..fabf3b2e2e 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -486,7 +486,8 @@ int dissect_image( #ifdef GPT_USR_NATIVE sd_id128_t usr_uuid = SD_ID128_NULL, usr_verity_uuid = SD_ID128_NULL; #endif - bool is_gpt, is_mbr, generic_rw, multiple_generic = false; + bool is_gpt, is_mbr, multiple_generic = false, + generic_rw = false; /* initialize to appease gcc */ _cleanup_(sd_device_unrefp) sd_device *d = NULL; _cleanup_(dissected_image_unrefp) DissectedImage *m = NULL; _cleanup_(blkid_free_probep) blkid_probe b = NULL; @@ -494,7 +495,7 @@ int dissect_image( sd_id128_t generic_uuid = SD_ID128_NULL; const char *pttype = NULL, *sysname = NULL; blkid_partlist pl; - int r, generic_nr, n_partitions; + int r, generic_nr = -1, n_partitions; struct stat st; usec_t deadline; @@ -1202,6 +1203,7 @@ int dissect_image( return -ENOMEM; } + assert(generic_nr >= 0); m->partitions[PARTITION_ROOT] = (DissectedPartition) { .found = true, .rw = generic_rw, From aff81b1851593ee8aa9875b72940affece0a7bfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 10:37:11 +0200 Subject: [PATCH 6/7] various: silence gcc warnings AFAICT, gcc is just being stupid in all those cases. --- src/basic/efivars.c | 2 +- src/home/homework-luks.c | 3 ++- src/libsystemd/sd-netlink/generic-netlink.c | 2 +- src/shared/sleep-config.c | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/basic/efivars.c b/src/basic/efivars.c index 26d34bb94f..2139cf3a69 100644 --- a/src/basic/efivars.c +++ b/src/basic/efivars.c @@ -63,7 +63,7 @@ int efi_get_variable( _cleanup_free_ char *p = NULL; _cleanup_free_ void *buf = NULL; struct stat st; - usec_t begin; + usec_t begin = 0; /* Unnecessary initialization to appease gcc */ uint32_t a; ssize_t n; diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 07d5bcfdb6..543195914f 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1870,7 +1870,8 @@ int home_create_luks( UserRecord **ret_home) { _cleanup_free_ char *dm_name = NULL, *dm_node = NULL, *subdir = NULL, *disk_uuid_path = NULL, *temporary_image_path = NULL; - uint64_t host_size, encrypted_size, partition_offset, partition_size; + uint64_t encrypted_size, + host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ bool image_created = false, dm_activated = false, mounted = false; _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; diff --git a/src/libsystemd/sd-netlink/generic-netlink.c b/src/libsystemd/sd-netlink/generic-netlink.c index 6760e0d348..a939d65569 100644 --- a/src/libsystemd/sd-netlink/generic-netlink.c +++ b/src/libsystemd/sd-netlink/generic-netlink.c @@ -118,7 +118,7 @@ static int lookup_id(sd_netlink *nl, sd_genl_family_t family, uint16_t *id) { } int sd_genl_message_new(sd_netlink *nl, sd_genl_family_t family, uint8_t cmd, sd_netlink_message **ret) { - uint16_t id; + uint16_t id = 0; /* Unnecessary initialization to appease gcc */ int r; r = lookup_id(nl, family, &id); diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index cea51482de..37f83306db 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -324,7 +324,7 @@ static bool location_is_resume_device(const HibernateLocation *location, dev_t s int find_hibernate_location(HibernateLocation **ret_hibernate_location) { _cleanup_fclose_ FILE *f = NULL; _cleanup_(hibernate_location_freep) HibernateLocation *hibernate_location = NULL; - dev_t sys_resume; + dev_t sys_resume = 0; /* Unnecessary initialization to appease gcc */ uint64_t sys_offset = 0; bool resume_match = false; int r; From e7d48709ed6c8569286463552f3df36bbdce8824 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 1 Apr 2021 11:12:57 +0200 Subject: [PATCH 7/7] resolved: avoid passing unitialized variable The issue was introduced in the refactoring in 775ae35403f8f3c01b7ac13387fe8aac1759993f. We would pass an initialized value to a helper function. We would only *use* it if it was initialized. But the mere passing of an unitialized variable is UB, so let's not do that. This silences a gcc warning. --- src/resolve/resolved-dns-cache.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index b74141e641..f73ead872d 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -938,6 +938,8 @@ static int answer_add_clamp_ttl( if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) { uint32_t left_ttl; + assert(current > 0); + /* Let's determine how much time is left for this cache entry. Note that we round down, but * clamp this to be 1s at minimum, since we usually want records to remain cached better too * short a time than too long a time, but otoh don't want to return 0 ever, since that has @@ -988,7 +990,7 @@ int dns_cache_lookup( bool nxdomain = false; DnsCacheItem *j, *first, *nsec = NULL; bool have_authenticated = false, have_non_authenticated = false, have_confidential = false, have_non_confidential = false; - usec_t current; + usec_t current = 0; int found_rcode = -1; DnssecResult dnssec_result = -1; int have_dnssec_result = -1; @@ -1014,8 +1016,12 @@ int dns_cache_lookup( goto miss; } - if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) + if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) { + /* 'current' is always passed to answer_add_clamp_ttl(), but is only used conditionally. + * We'll do the same assert there to make sure that it was initialized properly. */ current = now(clock_boottime_or_monotonic()); + assert(current > 0); + } LIST_FOREACH(by_key, j, first) { /* If the caller doesn't allow us to answer questions from cache data learned from