From 5600a26114d7c93592d3b030e0d02be62919ad51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 9 Apr 2021 14:42:39 +0200 Subject: [PATCH 01/20] bpf-devices: update comment --- src/core/bpf-devices.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/bpf-devices.c b/src/core/bpf-devices.c index 1ad7ade306..8a345a4498 100644 --- a/src/core/bpf-devices.c +++ b/src/core/bpf-devices.c @@ -216,7 +216,7 @@ int bpf_devices_apply_policy( _cleanup_free_ char *controller_path = NULL; int r; - /* This will assign *keep_program if everything goes well. */ + /* This will assign *prog_installed if everything goes well. */ if (!prog) goto finish; From 47350c5fb64e7c76e071a928bf9251dfe79d3ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 13:17:22 +0200 Subject: [PATCH 02/20] meson: simplify the BUILD_MODE conditional Using a enum is all nice and generic, but at this point it seems unlikely that we'll add further build modes. But having an enum means that we need to include the header file with the enumeration whenerever the conditional is used. I want to use the conditional in log.h, which makes it hard to avoid circular imports. --- meson.build | 8 ++++---- src/analyze/analyze.c | 2 +- src/basic/build.h | 5 ----- src/basic/missing_capability.h | 2 +- src/import/curl-util.c | 2 +- src/journal-remote/journal-upload.c | 2 +- src/libudev/test-libudev.c | 2 +- src/stdio-bridge/stdio-bridge.c | 2 +- src/test/test-cgroup-setup.c | 2 +- src/test/test-cgroup-util.c | 2 +- src/test/test-udev.c | 2 +- src/udev/dmi_memory_id/dmi_memory_id.c | 2 +- src/udev/scsi_id/scsi_id.c | 2 +- src/udev/udevadm.h | 1 - src/udev/udevd.c | 2 +- 15 files changed, 16 insertions(+), 22 deletions(-) diff --git a/meson.build b/meson.build index fa929370bd..7d9f2d3d4c 100644 --- a/meson.build +++ b/meson.build @@ -38,8 +38,8 @@ relative_source_path = run_command('realpath', project_source_root).stdout().strip() conf.set_quoted('RELATIVE_SOURCE_PATH', relative_source_path) -conf.set('BUILD_MODE', 'BUILD_MODE_' + get_option('mode').to_upper(), - description : 'tailor build to development or release builds') +conf.set10('BUILD_MODE_DEVELOPER', get_option('mode') == 'developer', + description : 'tailor build to development or release builds') want_ossfuzz = get_option('oss-fuzz') want_libfuzzer = get_option('llvm-fuzz') @@ -1117,7 +1117,7 @@ else libcurl = [] endif conf.set10('HAVE_LIBCURL', have) -conf.set10('CURL_NO_OLDIES', get_option('mode') == 'developer') +conf.set10('CURL_NO_OLDIES', conf.get('BUILD_MODE_DEVELOPER') == 1) want_libidn = get_option('libidn') want_libidn2 = get_option('libidn2') @@ -3640,7 +3640,7 @@ if dbus_docs.length() > 0 '@INPUT@'], input : dbus_docs) - if conf.get('BUILD_MODE') == 'BUILD_MODE_DEVELOPER' + if conf.get('BUILD_MODE_DEVELOPER') == 1 test('dbus-docs-fresh', update_dbus_docs_py, args : ['--build-dir=@0@'.format(project_build_root), diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 1a38d878a3..1ad3731852 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -15,7 +15,6 @@ #include "analyze-condition.h" #include "analyze-security.h" #include "analyze-verify.h" -#include "build.h" #include "bus-error.h" #include "bus-locator.h" #include "bus-map-properties.h" @@ -53,6 +52,7 @@ #include "unit-name.h" #include "util.h" #include "verbs.h" +#include "version.h" #define SCALE_X (0.1 / 1000.0) /* pixels per us */ #define SCALE_Y (20.0) diff --git a/src/basic/build.h b/src/basic/build.h index 3de0d36cc9..87276bf686 100644 --- a/src/basic/build.h +++ b/src/basic/build.h @@ -4,8 +4,3 @@ #include "version.h" extern const char* const systemd_features; - -enum { - BUILD_MODE_DEVELOPER, - BUILD_MODE_RELEASE, -}; diff --git a/src/basic/missing_capability.h b/src/basic/missing_capability.h index 4cf31cb839..5adda554e5 100644 --- a/src/basic/missing_capability.h +++ b/src/basic/missing_capability.h @@ -27,7 +27,7 @@ #ifdef CAP_LAST_CAP # if CAP_LAST_CAP > SYSTEMD_CAP_LAST_CAP -# if BUILD_MODE == BUILD_MODE_DEVELOPER && defined(TEST_CAPABILITY_C) +# if BUILD_MODE_DEVELOPER && defined(TEST_CAPABILITY_C) # warning "The capability list here is outdated" # endif # else diff --git a/src/import/curl-util.c b/src/import/curl-util.c index e6db810635..ed2ac0a654 100644 --- a/src/import/curl-util.c +++ b/src/import/curl-util.c @@ -3,11 +3,11 @@ #include #include "alloc-util.h" -#include "build.h" #include "curl-util.h" #include "fd-util.h" #include "locale-util.h" #include "string-util.h" +#include "version.h" static void curl_glue_check_finished(CurlGlue *g) { CURLMsg *msg; diff --git a/src/journal-remote/journal-upload.c b/src/journal-remote/journal-upload.c index a8f1f7e511..d7e45364a6 100644 --- a/src/journal-remote/journal-upload.c +++ b/src/journal-remote/journal-upload.c @@ -9,7 +9,6 @@ #include "sd-daemon.h" #include "alloc-util.h" -#include "build.h" #include "conf-parser.h" #include "daemon-util.h" #include "def.h" @@ -34,6 +33,7 @@ #include "strv.h" #include "tmpfile-util.h" #include "util.h" +#include "version.h" #define PRIV_KEY_FILE CERTIFICATE_ROOT "/private/journal-upload.pem" #define CERT_FILE CERTIFICATE_ROOT "/certs/journal-upload.pem" diff --git a/src/libudev/test-libudev.c b/src/libudev/test-libudev.c index 12bd0d6299..a751056795 100644 --- a/src/libudev/test-libudev.c +++ b/src/libudev/test-libudev.c @@ -6,7 +6,6 @@ #include #include "alloc-util.h" -#include "build.h" #include "fd-util.h" #include "libudev-list-internal.h" #include "libudev-util.h" @@ -15,6 +14,7 @@ #include "stdio-util.h" #include "string-util.h" #include "tests.h" +#include "version.h" static bool arg_monitor = false; diff --git a/src/stdio-bridge/stdio-bridge.c b/src/stdio-bridge/stdio-bridge.c index 217bd97ea5..b45f7912cb 100644 --- a/src/stdio-bridge/stdio-bridge.c +++ b/src/stdio-bridge/stdio-bridge.c @@ -10,7 +10,6 @@ #include "sd-daemon.h" #include "alloc-util.h" -#include "build.h" #include "bus-internal.h" #include "bus-util.h" #include "errno-util.h" @@ -18,6 +17,7 @@ #include "log.h" #include "main-func.h" #include "util.h" +#include "version.h" #define DEFAULT_BUS_PATH "unix:path=/run/dbus/system_bus_socket" diff --git a/src/test/test-cgroup-setup.c b/src/test/test-cgroup-setup.c index 4978a92e46..37ef66b0fd 100644 --- a/src/test/test-cgroup-setup.c +++ b/src/test/test-cgroup-setup.c @@ -3,13 +3,13 @@ #include #include "alloc-util.h" -#include "build.h" #include "cgroup-setup.h" #include "errno-util.h" #include "log.h" #include "proc-cmdline.h" #include "string-util.h" #include "tests.h" +#include "version.h" static void test_is_wanted_print(bool header) { _cleanup_free_ char *cmdline = NULL; diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index f95832acf6..c2adfa07ce 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include "alloc-util.h" -#include "build.h" #include "cgroup-util.h" #include "dirent-util.h" #include "errno-util.h" @@ -17,6 +16,7 @@ #include "tests.h" #include "user-util.h" #include "util.h" +#include "version.h" static void check_p_d_u(const char *path, int code, const char *result) { _cleanup_free_ char *unit = NULL; diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 488b965c82..6bb8a9e4fc 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -11,7 +11,6 @@ #include #include -#include "build.h" #include "device-private.h" #include "fs-util.h" #include "log.h" @@ -24,6 +23,7 @@ #include "string-util.h" #include "tests.h" #include "udev-event.h" +#include "version.h" static int fake_filesystems(void) { static const struct fakefs { diff --git a/src/udev/dmi_memory_id/dmi_memory_id.c b/src/udev/dmi_memory_id/dmi_memory_id.c index c5bea8c9a8..64eba0d314 100644 --- a/src/udev/dmi_memory_id/dmi_memory_id.c +++ b/src/udev/dmi_memory_id/dmi_memory_id.c @@ -45,12 +45,12 @@ #include #include "alloc-util.h" -#include "build.h" #include "fileio.h" #include "main-func.h" #include "string-util.h" #include "udev-util.h" #include "unaligned.h" +#include "version.h" #define SUPPORTED_SMBIOS_VER 0x030300 diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index d9d897c00c..41f92b68be 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -17,7 +17,6 @@ #include #include "alloc-util.h" -#include "build.h" #include "device-nodes.h" #include "extract-word.h" #include "fd-util.h" @@ -27,6 +26,7 @@ #include "strv.h" #include "strxcpyx.h" #include "udev-util.h" +#include "version.h" static const struct option options[] = { { "device", required_argument, NULL, 'd' }, diff --git a/src/udev/udevadm.h b/src/udev/udevadm.h index 162bbb9a43..75ce633632 100644 --- a/src/udev/udevadm.h +++ b/src/udev/udevadm.h @@ -3,7 +3,6 @@ #include -#include "build.h" #include "macro.h" int info_main(int argc, char *argv[], void *userdata); diff --git a/src/udev/udevd.c b/src/udev/udevd.c index 0a44b40a32..2c702d0388 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -28,7 +28,6 @@ #include "sd-event.h" #include "alloc-util.h" -#include "build.h" #include "cgroup-util.h" #include "cpu-set-util.h" #include "dev-setup.h" @@ -65,6 +64,7 @@ #include "udev-util.h" #include "udev-watch.h" #include "user-util.h" +#include "version.h" #define WORKER_NUM_MAX 2048U From 2d359acda53280489c48d7aee84ba6d53e30a615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 04:55:09 -0400 Subject: [PATCH 03/20] libsystemd-network: fix dhcp option buffer confusion We were writing to the wrong buffer with a wrong offset :( Bug present since the original introduction of the code in 04b28be1a306fd2ba454d3ee333d63df71aa3873. --- src/libsystemd-network/dhcp-option.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libsystemd-network/dhcp-option.c b/src/libsystemd-network/dhcp-option.c index faa075cbd7..8899e8a552 100644 --- a/src/libsystemd-network/dhcp-option.c +++ b/src/libsystemd-network/dhcp-option.c @@ -17,6 +17,7 @@ static int option_append(uint8_t options[], size_t size, size_t *offset, uint8_t code, size_t optlen, const void *optval) { assert(options); + assert(size > 0); assert(offset); if (code != SD_DHCP_OPTION_END) @@ -165,7 +166,7 @@ int dhcp_option_append(DHCPMessage *message, size_t size, size_t *offset, } else if (r == -ENOBUFS && use_sname) { /* did not fit, but we have more buffers to try close the file array and move the offset to its end */ - r = option_append(message->options, size, offset, SD_DHCP_OPTION_END, 0, NULL); + r = option_append(message->file, sizeof(message->file), &file_offset, SD_DHCP_OPTION_END, 0, NULL); if (r < 0) return r; From aca591ac55e5ee364905aec975388c5e30d0476c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 05:54:17 -0400 Subject: [PATCH 04/20] sd-device: improve log message and tweak style We shouldn't say the attribute is missing right after ruling out ENOENT. --- src/libsystemd/sd-device/sd-device.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index d9ea9852e3..927a4f1d52 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -178,13 +178,12 @@ int device_set_syspath(sd_device *device, const char *_syspath, bool verify) { /* all 'devices' require an 'uevent' file */ path = strjoina(syspath, "/uevent"); - r = access(path, F_OK); - if (r < 0) { + if (access(path, F_OK) < 0) { if (errno == ENOENT) /* this is not a valid device */ return -ENODEV; - return log_debug_errno(errno, "sd-device: %s does not have an uevent file: %m", syspath); + return log_debug_errno(errno, "sd-device: cannot access uevent file for %s: %m", syspath); } } else { /* everything else just needs to be a directory */ From 111a3aae71fa019710216cc5b7aa95b7c8db0937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 06:14:01 -0400 Subject: [PATCH 05/20] partition, random-seed, logind: fix log messages with %m We would print "...: Success", which is not too terrible, but not pretty either. --- src/login/logind.c | 4 ++-- src/partition/repart.c | 2 +- src/random-seed/random-seed.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 4887889fac..f461f03ec4 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -268,8 +268,8 @@ static int manager_enumerate_seats(Manager *m) { s = hashmap_get(m->seats, de->d_name); if (!s) { if (unlinkat(dirfd(d), de->d_name, 0) < 0) - log_warning("Failed to remove /run/systemd/seats/%s: %m", - de->d_name); + log_warning_errno(errno, "Failed to remove /run/systemd/seats/%s: %m", + de->d_name); continue; } diff --git a/src/partition/repart.c b/src/partition/repart.c index 195a909bf3..04d5bb18f4 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -3830,7 +3830,7 @@ static int parse_efi_variable_factory_reset(void) { arg_factory_reset = r; if (r) - log_notice("Honouring factory reset requested via EFI variable FactoryReset: %m"); + log_notice("Factory reset requested via EFI variable FactoryReset."); return 0; } diff --git a/src/random-seed/random-seed.c b/src/random-seed/random-seed.c index 4caf967807..e003ca60e3 100644 --- a/src/random-seed/random-seed.c +++ b/src/random-seed/random-seed.c @@ -262,7 +262,7 @@ static int run(int argc, char *argv[]) { if (k < 0) log_debug_errno(errno, "Failed to read random data with getrandom(), falling back to /dev/urandom: %m"); else if ((size_t) k < buf_size) - log_debug("Short read from getrandom(), falling back to /dev/urandom: %m"); + log_debug("Short read from getrandom(), falling back to /dev/urandom."); else getrandom_worked = true; From cbe97b9c92728e96c9987314a7e1e05bbdffda16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 06:05:47 -0400 Subject: [PATCH 06/20] basic/log: force log_*_errno() to return negative This silences some warnigns where gcc thinks that some variables are unitialized. One particular case: ../src/journal/journald-server.c: In function 'ache_space_refresh': ../src/journal/journald-server.c:136:28: error: 'vfs_avail' may be used uninitialized in this function [-Werror=maybe-uninitialized] 136 | uint64_t vfs_used, vfs_avail, avail; | ^~~~~~~~~ ../src/journal/journald-server.c:136:18: error: 'vfs_used' may be used uninitialized in this function [-Werror=maybe-uninitialized] 136 | uint64_t vfs_used, vfs_avail, avail; | ^~~~~~~~ cc1: all warnings being treated as errors which is caused by d = opendir(path); if (!d) return log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_ERR, errno, "Failed to open %s: %m", path); if (fstatvfs(dirfd(d), &ss) < 0) return log_error_errno(errno, "Failed to fstatvfs(%s): %m", path); For some reason on aarch64 gcc thinks we might return non-negative here. In principle errno must be set in both cases, but it's hard to say for certain. So let's make sure that our code flow is correct, even if somebody forgot to set the global variable somewhere. --- src/basic/log.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/basic/log.h b/src/basic/log.h index b3d32abfc8..4b621097d4 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -191,9 +191,10 @@ void log_assert_failed_return( #define log_full_errno(level, error, ...) \ ({ \ int _level = (level), _e = (error); \ - (log_get_max_level() >= LOG_PRI(_level)) \ + _e = (log_get_max_level() >= LOG_PRI(_level)) \ ? log_internal(_level, _e, PROJECT_FILE, __LINE__, __func__, __VA_ARGS__) \ : -ERRNO_VALUE(_e); \ + _e < 0 ? _e : -EIO; \ }) #define log_full(level, ...) (void) log_full_errno((level), 0, __VA_ARGS__) From 63275a70323f2535aa943e47e827ec65a7358f0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 13:39:14 +0200 Subject: [PATCH 07/20] basic/log: assert that %m is not used when error is not set This is only done in developer mode. It is a pretty rare occurence that we make this kind of mistake. And even if it happens, the result is just a misleading error message. So let's only do the check in non-release builds. --- src/basic/log.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/basic/log.h b/src/basic/log.h index 4b621097d4..f7785b016b 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "macro.h" @@ -197,7 +198,12 @@ void log_assert_failed_return( _e < 0 ? _e : -EIO; \ }) -#define log_full(level, ...) (void) log_full_errno((level), 0, __VA_ARGS__) +#define log_full(level, fmt, ...) \ + ({ \ + if (BUILD_MODE_DEVELOPER) \ + assert(!strstr(fmt, "%m")); \ + (void) log_full_errno((level), 0, fmt, ##__VA_ARGS__); \ + }) int log_emergency_level(void); From a626cb15c0a029270779887dc3bc12e2dcadd273 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 15:55:16 +0200 Subject: [PATCH 08/20] basic/log: assert that 0 is not passed as errno, except in test code Let's assert if we ever happen to pass 0 to one of the log functions. With the preceding commit to return -EIO from log_*(), passing 0 wouldn't affect the return value any more, but it is still most likely an error. The unit test code is an exception: we fairly often pass the return value to print it, before checking what it is. So let's assert that we're not passing 0 in non-test code. As with the previous check for %m, this is only done in developer mode. We are depending on external code setting errno correctly for us, which might not always be true, and which we can't test, so we shouldn't assert, but just handle this gracefully. I did a bunch of greps to try to figure out if there are any places where we're passing 0 on purpose, and couldn't find any. The one place that failed in tests is adjusted. About "zerook" in the name: I wanted the suffix to be unambiguous. It's a single "word" because each of the words in log_full_errno is also meaningful, and having one term use two words would be confusing. --- meson.build | 22 +++++++++++++--------- src/basic/log.h | 19 ++++++++++++++++--- src/shared/bus-wait-for-jobs.c | 3 ++- src/test/test-bpf-foreign-programs.c | 2 +- 4 files changed, 32 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 7d9f2d3d4c..8b4458d205 100644 --- a/meson.build +++ b/meson.build @@ -3318,11 +3318,15 @@ custom_target( '} >@OUTPUT@'], build_by_default : true) -# We intentionally do not do inline initializations with definitions for -# a bunch of _cleanup_ variables in tests, to ensure valgrind is triggered. -# This triggers a lot of maybe-uninitialized false positives when the -# combination of -O2 and -flto is used. Suppress them. -no_uninit = '-O2' in get_option('c_args') and '-flto=auto' in get_option('c_args') ? cc.first_supported_argument('-Wno-maybe-uninitialized') : [] +test_cflags = ['-DTEST_CODE=1'] +# We intentionally do not do inline initializations with definitions for a +# bunch of _cleanup_ variables in tests, to ensure valgrind is triggered if we +# use the variable unexpectedly. This triggers a lot of maybe-uninitialized +# false positives when the combination of -O2 and -flto is used. Suppress them. +if '-O2' in get_option('c_args') and '-flto=auto' in get_option('c_args') + test_cflags += cc.first_supported_argument('-Wno-maybe-uninitialized') +endif + foreach tuple : tests sources = tuple[0] link_with = tuple.length() > 1 and tuple[1].length() > 0 ? tuple[1] : [libshared] @@ -3331,7 +3335,7 @@ foreach tuple : tests condition = tuple.length() > 4 ? tuple[4] : '' type = tuple.length() > 5 ? tuple[5] : '' defs = tuple.length() > 6 ? tuple[6] : [] - defs += no_uninit + defs += test_cflags parallel = tuple.length() > 7 ? tuple[7] : true timeout = 30 @@ -3399,7 +3403,7 @@ exe = executable( 'test-libudev-sym', test_libudev_sym_c, include_directories : libudev_includes, - c_args : ['-Wno-deprecated-declarations'] + no_uninit, + c_args : ['-Wno-deprecated-declarations'] + test_cflags, link_with : [libudev], build_by_default : want_tests != 'false', install : install_tests, @@ -3412,7 +3416,7 @@ exe = executable( 'test-libudev-static-sym', test_libudev_sym_c, include_directories : libudev_includes, - c_args : ['-Wno-deprecated-declarations'] + no_uninit, + c_args : ['-Wno-deprecated-declarations'] + test_cflags, link_with : [install_libudev_static], build_by_default : want_tests != 'false' and static_libudev_pic, install : install_tests and static_libudev_pic, @@ -3453,7 +3457,7 @@ foreach tuple : fuzzers include_directories : [incs, include_directories('src/fuzz')], link_with : link_with, dependencies : dependencies, - c_args : defs, + c_args : defs + test_cflags, link_args: link_args, install : false, build_by_default : fuzz_tests or fuzzer_build) diff --git a/src/basic/log.h b/src/basic/log.h index f7785b016b..9a57fcd0f1 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -189,7 +189,7 @@ void log_assert_failed_return( log_dispatch_internal(level, error, PROJECT_FILE, __LINE__, __func__, NULL, NULL, NULL, NULL, buffer) /* Logging with level */ -#define log_full_errno(level, error, ...) \ +#define log_full_errno_zerook(level, error, ...) \ ({ \ int _level = (level), _e = (error); \ _e = (log_get_max_level() >= LOG_PRI(_level)) \ @@ -198,17 +198,30 @@ void log_assert_failed_return( _e < 0 ? _e : -EIO; \ }) +#if BUILD_MODE_DEVELOPER && !defined(TEST_CODE) +# define ASSERT_NON_ZERO(x) assert((x) != 0) +#else +# define ASSERT_NON_ZERO(x) +#endif + +#define log_full_errno(level, error, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_full_errno_zerook(level, _error, __VA_ARGS__); \ + }) + #define log_full(level, fmt, ...) \ ({ \ if (BUILD_MODE_DEVELOPER) \ assert(!strstr(fmt, "%m")); \ - (void) log_full_errno((level), 0, fmt, ##__VA_ARGS__); \ + (void) log_full_errno_zerook(level, 0, fmt, ##__VA_ARGS__); \ }) int log_emergency_level(void); /* Normal logging */ -#define log_debug(...) log_full_errno(LOG_DEBUG, 0, __VA_ARGS__) +#define log_debug(...) log_full_errno_zerook(LOG_DEBUG, 0, __VA_ARGS__) #define log_info(...) log_full(LOG_INFO, __VA_ARGS__) #define log_notice(...) log_full(LOG_NOTICE, __VA_ARGS__) #define log_warning(...) log_full(LOG_WARNING, __VA_ARGS__) diff --git a/src/shared/bus-wait-for-jobs.c b/src/shared/bus-wait-for-jobs.c index e66c8beafa..8458fe8684 100644 --- a/src/shared/bus-wait-for-jobs.c +++ b/src/shared/bus-wait-for-jobs.c @@ -306,7 +306,8 @@ int bus_wait_for_jobs(BusWaitForJobs *d, bool quiet, const char* const* extra_ar if (q < 0 && r == 0) r = q; - log_debug_errno(q, "Got result %s/%m for job %s", d->result, d->name); + log_full_errno_zerook(LOG_DEBUG, q, + "Got result %s/%m for job %s", d->result, d->name); } d->name = mfree(d->name); diff --git a/src/test/test-bpf-foreign-programs.c b/src/test/test-bpf-foreign-programs.c index e703924077..666317e520 100644 --- a/src/test/test-bpf-foreign-programs.c +++ b/src/test/test-bpf-foreign-programs.c @@ -293,7 +293,7 @@ int main(int argc, char *argv[]) { r = cg_all_unified(); if (r <= 0) - return log_tests_skipped_errno(r, "Unified hierarchy is required, skipping."); + return log_tests_skipped("Unified hierarchy is required, skipping."); r = enter_cgroup_subroot(NULL); if (r == -ENOMEDIUM) From a2eb2267e44580446ecad37e7206e729cfd78155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 17:10:36 +0200 Subject: [PATCH 09/20] shared/module-util: fix errno value passed to log function If r == 0, no harm done. But if r > 0, this would be interpreted as an errno value, wrongly. --- src/shared/module-util.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/shared/module-util.c b/src/shared/module-util.c index 587e6369fb..1526f59b0a 100644 --- a/src/shared/module-util.c +++ b/src/shared/module-util.c @@ -20,11 +20,10 @@ int module_load_and_warn(struct kmod_ctx *ctx, const char *module, bool verbose) return log_full_errno(verbose ? LOG_ERR : LOG_DEBUG, r, "Failed to look up module alias '%s': %m", module); - if (!modlist) { - log_full_errno(verbose ? LOG_ERR : LOG_DEBUG, r, - "Failed to find module '%s'", module); - return -ENOENT; - } + if (!modlist) + return log_full_errno(verbose ? LOG_ERR : LOG_DEBUG, + SYNTHETIC_ERRNO(ENOENT), + "Failed to find module '%s'", module); kmod_list_foreach(itr, modlist) { _cleanup_(kmod_module_unrefp) struct kmod_module *mod = NULL; From 75029e150b47543b5c352298170847d39489c3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 17:44:47 +0200 Subject: [PATCH 10/20] Do not try to return 0 from log_debug() As @yuwata correctly points out, this became broken when log_debug() started returning -EIO. I wanted to preserve this pattern, but it turns out it is not very widely used, and preserving it would make the whole thing, already quite complicated, even more complex. log_debug() is made like log_info() and friends, and returns void. --- src/basic/log.h | 2 +- src/sleep/sleep.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/basic/log.h b/src/basic/log.h index 9a57fcd0f1..81330166a4 100644 --- a/src/basic/log.h +++ b/src/basic/log.h @@ -221,7 +221,7 @@ void log_assert_failed_return( int log_emergency_level(void); /* Normal logging */ -#define log_debug(...) log_full_errno_zerook(LOG_DEBUG, 0, __VA_ARGS__) +#define log_debug(...) log_full(LOG_DEBUG, __VA_ARGS__) #define log_info(...) log_full(LOG_INFO, __VA_ARGS__) #define log_notice(...) log_full(LOG_NOTICE, __VA_ARGS__) #define log_warning(...) log_full(LOG_WARNING, __VA_ARGS__) diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 262d4cea66..8aeaa1a543 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -163,10 +163,10 @@ static int lock_all_homes(void) { if (!bus_error_is_unknown_service(&error)) return log_error_errno(r, "Failed to lock home directories: %s", bus_error_message(&error, r)); - return log_debug("systemd-homed is not running, locking of home directories skipped."); - } - - return log_debug("Successfully requested locking of all home directories."); + log_debug("systemd-homed is not running, locking of home directories skipped."); + } else + log_debug("Successfully requested locking of all home directories."); + return 0; } static int execute(char **modes, char **states, const char *action) { From cf5a2ee82517429a34d9f5bef853cabe055e3e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 22:06:02 +0200 Subject: [PATCH 11/20] journald: fix %m usage --- src/journal/journald-kmsg.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/journal/journald-kmsg.c b/src/journal/journald-kmsg.c index e7255b0355..29b8437fe2 100644 --- a/src/journal/journald-kmsg.c +++ b/src/journal/journald-kmsg.c @@ -376,8 +376,8 @@ int server_open_dev_kmsg(Server *s) { s->dev_kmsg_fd = open("/dev/kmsg", mode); if (s->dev_kmsg_fd < 0) { - log_full(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, - "Failed to open /dev/kmsg, ignoring: %m"); + log_full_errno(errno == ENOENT ? LOG_DEBUG : LOG_WARNING, + errno, "Failed to open /dev/kmsg, ignoring: %m"); return 0; } From e89f6ed476ba488bc4aff3d428725baee4d5e5e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 22:19:58 +0200 Subject: [PATCH 12/20] Voidify log_link_debug See analogous change for log_debug() for discussion. --- src/network/networkd-mdb.c | 12 ++++++++---- src/network/networkd-nexthop.c | 6 ++++-- src/shared/log-link.h | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/network/networkd-mdb.c b/src/network/networkd-mdb.c index f5aff72248..b3d583e6e6 100644 --- a/src/network/networkd-mdb.c +++ b/src/network/networkd-mdb.c @@ -208,8 +208,10 @@ int link_set_bridge_mdb(Link *link) { if (hashmap_isempty(link->network->mdb_entries_by_section)) goto finish; - if (!link_has_carrier(link)) - return log_link_debug(link, "Link does not have carrier yet, setting MDB entries later."); + if (!link_has_carrier(link)) { + log_link_debug(link, "Link does not have carrier yet, setting MDB entries later."); + return 0; + } if (link->network->bridge) { Link *master; @@ -218,8 +220,10 @@ int link_set_bridge_mdb(Link *link) { if (r < 0) return log_link_error_errno(link, r, "Failed to get Link object for Bridge=%s", link->network->bridge->ifname); - if (!link_has_carrier(master)) - return log_link_debug(link, "Bridge interface %s does not have carrier yet, setting MDB entries later.", link->network->bridge->ifname); + if (!link_has_carrier(master)) { + log_link_debug(link, "Bridge interface %s does not have carrier yet, setting MDB entries later.", link->network->bridge->ifname); + return 0; + } } else if (!streq_ptr(link->kind, "bridge")) { log_link_warning(link, "Link is neither a bridge master nor a bridge port, ignoring [BridgeMDB] sections."); diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index 470095c897..c32cc70798 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -711,8 +711,10 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, if (r < 0) { log_link_warning_errno(link, r, "rtnl: could not get nexthop family, ignoring: %m"); return 0; - } else if (!IN_SET(tmp->family, AF_INET, AF_INET6)) - return log_link_debug(link, "rtnl: received nexthop message with invalid family %d, ignoring.", tmp->family); + } else if (!IN_SET(tmp->family, AF_INET, AF_INET6)) { + log_link_debug(link, "rtnl: received nexthop message with invalid family %d, ignoring.", tmp->family); + return 0; + } r = sd_rtnl_message_nexthop_get_protocol(message, &tmp->protocol); if (r < 0) { diff --git a/src/shared/log-link.h b/src/shared/log-link.h index 3a4dcaa267..ecc2609e51 100644 --- a/src/shared/log-link.h +++ b/src/shared/log-link.h @@ -29,7 +29,7 @@ #define log_link_full(link, level, ...) (void) log_link_full_errno(link, level, 0, __VA_ARGS__) -#define log_link_debug(link, ...) log_link_full_errno(link, LOG_DEBUG, 0, __VA_ARGS__) +#define log_link_debug(link, ...) log_link_full(link, LOG_DEBUG, __VA_ARGS__) #define log_link_info(link, ...) log_link_full(link, LOG_INFO, __VA_ARGS__) #define log_link_notice(link, ...) log_link_full(link, LOG_NOTICE, __VA_ARGS__) #define log_link_warning(link, ...) log_link_full(link, LOG_WARNING, __VA_ARGS__) From f407219cd1e70a62a7c2596d75bd7edcc4df697c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 22:53:49 +0200 Subject: [PATCH 13/20] Check that errno passed log_{interface,link}_*_errno() is non-zero --- src/libsystemd-network/dhcp-internal.h | 2 +- src/libsystemd-network/dhcp-server-internal.h | 2 +- src/libsystemd-network/dhcp6-internal.h | 2 +- src/libsystemd-network/lldp-internal.h | 2 +- src/libsystemd-network/ndisc-internal.h | 2 +- src/libsystemd-network/radv-internal.h | 2 +- src/libsystemd-network/sd-ipv4acd.c | 2 +- src/libsystemd-network/sd-ipv4ll.c | 2 +- src/shared/log-link.h | 22 +++++++++++++++---- 9 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h index c5c851c575..8eefc02726 100644 --- a/src/libsystemd-network/dhcp-internal.h +++ b/src/libsystemd-network/dhcp-internal.h @@ -70,7 +70,7 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum, ui ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_dhcp_client_get_ifname(client), \ LOG_DEBUG, _e, "DHCPv4 client: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index d30d91082f..391f2da9ac 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -102,7 +102,7 @@ int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b); ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_dhcp_server_get_ifname(server), \ LOG_DEBUG, _e, "DHCPv4 server: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 274b14b056..26897404fe 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -123,7 +123,7 @@ int dhcp6_message_status_from_string(const char *s) _pure_; ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_dhcp6_client_get_ifname(client), \ LOG_DEBUG, _e, "DHCPv6 client: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/lldp-internal.h b/src/libsystemd-network/lldp-internal.h index f13555d35c..6babcce066 100644 --- a/src/libsystemd-network/lldp-internal.h +++ b/src/libsystemd-network/lldp-internal.h @@ -40,7 +40,7 @@ sd_lldp_event_t lldp_event_from_string(const char *s) _pure_; ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_lldp_get_ifname(lldp), \ LOG_DEBUG, _e, "LLDP: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/ndisc-internal.h b/src/libsystemd-network/ndisc-internal.h index 44a7e76c21..d7aa69a181 100644 --- a/src/libsystemd-network/ndisc-internal.h +++ b/src/libsystemd-network/ndisc-internal.h @@ -45,7 +45,7 @@ sd_ndisc_event_t ndisc_event_from_string(const char *s) _pure_; ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_ndisc_get_ifname(ndisc), \ LOG_DEBUG, _e, "NDISC: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/radv-internal.h b/src/libsystemd-network/radv-internal.h index fe5d74fda4..eef6db514a 100644 --- a/src/libsystemd-network/radv-internal.h +++ b/src/libsystemd-network/radv-internal.h @@ -129,7 +129,7 @@ struct sd_radv_route_prefix { ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_radv_get_ifname(radv), \ LOG_DEBUG, _e, "RADV: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index 643bdc4ba9..65a8588725 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -79,7 +79,7 @@ struct sd_ipv4acd { ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_ipv4acd_get_ifname(acd), \ LOG_DEBUG, _e, "IPv4ACD: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c index 49e9350fba..cdde412a55 100644 --- a/src/libsystemd-network/sd-ipv4ll.c +++ b/src/libsystemd-network/sd-ipv4ll.c @@ -53,7 +53,7 @@ struct sd_ipv4ll { ({ \ int _e = (error); \ if (DEBUG_LOGGING) \ - log_interface_full_errno( \ + log_interface_full_errno_zerook( \ sd_ipv4ll_get_ifname(ll), \ LOG_DEBUG, _e, "IPv4LL: " fmt, \ ##__VA_ARGS__); \ diff --git a/src/shared/log-link.h b/src/shared/log-link.h index ecc2609e51..5f2b176353 100644 --- a/src/shared/log-link.h +++ b/src/shared/log-link.h @@ -3,13 +3,20 @@ #include "log.h" -#define log_interface_full_errno(ifname, level, error, ...) \ +#define log_interface_full_errno_zerook(ifname, level, error, ...) \ ({ \ const char *_ifname = (ifname); \ _ifname ? log_object_internal(level, error, PROJECT_FILE, __LINE__, __func__, "INTERFACE=", _ifname, NULL, NULL, ##__VA_ARGS__) : \ log_internal(level, error, PROJECT_FILE, __LINE__, __func__, ##__VA_ARGS__); \ }) +#define log_interface_full_errno(ifname, level, error, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_interface_full_errno_zerook(ifname, level, _error, __VA_ARGS__); \ + }) + /* * The following macros append INTERFACE= to the message. * The macros require a struct named 'Link' which contains 'char *ifname': @@ -21,13 +28,20 @@ * See, network/networkd-link.h for example. */ -#define log_link_full_errno(link, level, error, ...) \ +#define log_link_full_errno_zerook(link, level, error, ...) \ ({ \ const Link *_l = (link); \ - log_interface_full_errno(_l ? _l->ifname : NULL, level, error, ##__VA_ARGS__); \ + log_interface_full_errno_zerook(_l ? _l->ifname : NULL, level, error, __VA_ARGS__); \ }) -#define log_link_full(link, level, ...) (void) log_link_full_errno(link, level, 0, __VA_ARGS__) +#define log_link_full_errno(link, level, error, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_link_full_errno_zerook(link, level, _error, __VA_ARGS__); \ + }) + +#define log_link_full(link, level, ...) (void) log_link_full_errno_zerook(link, level, 0, __VA_ARGS__) #define log_link_debug(link, ...) log_link_full(link, LOG_DEBUG, __VA_ARGS__) #define log_link_info(link, ...) log_link_full(link, LOG_INFO, __VA_ARGS__) From a0c2541b6bd38fcfd8282a5f86d6366fbd20d614 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 23:12:50 +0200 Subject: [PATCH 14/20] =?UTF-8?q?libsystemd-network:=20use=20macro=20for?= =?UTF-8?q?=20definitions=20of=20log=5F{lldp,dhcp,=E2=80=A6}=5Ferrno?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No functional change. --- src/libsystemd-network/dhcp-internal.h | 15 +++++---------- src/libsystemd-network/dhcp-server-internal.h | 15 +++++---------- src/libsystemd-network/dhcp6-internal.h | 17 ++++++----------- src/libsystemd-network/lldp-internal.h | 17 ++++++----------- src/libsystemd-network/ndisc-internal.h | 17 ++++++----------- src/libsystemd-network/radv-internal.h | 17 ++++++----------- src/libsystemd-network/sd-ipv4acd.c | 15 +++++---------- src/libsystemd-network/sd-ipv4ll.c | 15 +++++---------- src/shared/log-link.h | 11 +++++++++++ 9 files changed, 55 insertions(+), 84 deletions(-) diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h index 8eefc02726..ee2ce2b250 100644 --- a/src/libsystemd-network/dhcp-internal.h +++ b/src/libsystemd-network/dhcp-internal.h @@ -66,15 +66,10 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum, ui #define DHCP_CLIENT_DONT_DESTROY(client) \ _cleanup_(sd_dhcp_client_unrefp) _unused_ sd_dhcp_client *_dont_destroy_##client = sd_dhcp_client_ref(client) -#define log_dhcp_client_errno(client, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_dhcp_client_get_ifname(client), \ - LOG_DEBUG, _e, "DHCPv4 client: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) +#define log_dhcp_client_errno(client, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "DHCPv4 client: ", \ + sd_dhcp_client_get_ifname(client), \ + error, fmt, ##__VA_ARGS__) #define log_dhcp_client(client, fmt, ...) \ log_dhcp_client_errno(client, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index 391f2da9ac..67c1cf4605 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -98,15 +98,10 @@ int dhcp_server_send_packet(sd_dhcp_server *server, void client_id_hash_func(const DHCPClientId *p, struct siphash *state); int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b); -#define log_dhcp_server_errno(server, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_dhcp_server_get_ifname(server), \ - LOG_DEBUG, _e, "DHCPv4 server: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) +#define log_dhcp_server_errno(server, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "DHCPv4 server: ", \ + sd_dhcp_server_get_ifname(server), \ + error, fmt, ##__VA_ARGS__) #define log_dhcp_server(server, fmt, ...) \ log_dhcp_server_errno(server, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 26897404fe..2b61a7bb3b 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -119,15 +119,10 @@ int dhcp6_message_type_from_string(const char *s) _pure_; const char *dhcp6_message_status_to_string(int s) _const_; int dhcp6_message_status_from_string(const char *s) _pure_; -#define log_dhcp6_client_errno(client, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_dhcp6_client_get_ifname(client), \ - LOG_DEBUG, _e, "DHCPv6 client: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) -#define log_dhcp6_client(client, fmt, ...) \ +#define log_dhcp6_client_errno(client, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "DHCPv6 client: ", \ + sd_dhcp6_client_get_ifname(client), \ + error, fmt, ##__VA_ARGS__) +#define log_dhcp6_client(client, fmt, ...) \ log_dhcp6_client_errno(client, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/lldp-internal.h b/src/libsystemd-network/lldp-internal.h index 6babcce066..8846fa5323 100644 --- a/src/libsystemd-network/lldp-internal.h +++ b/src/libsystemd-network/lldp-internal.h @@ -36,15 +36,10 @@ struct sd_lldp { const char* lldp_event_to_string(sd_lldp_event_t e) _const_; sd_lldp_event_t lldp_event_from_string(const char *s) _pure_; -#define log_lldp_errno(lldp, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_lldp_get_ifname(lldp), \ - LOG_DEBUG, _e, "LLDP: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) -#define log_lldp(lldp, fmt, ...) \ +#define log_lldp_errno(lldp, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "LLDP: ", \ + sd_lldp_get_ifname(lldp), \ + error, fmt, ##__VA_ARGS__) +#define log_lldp(lldp, fmt, ...) \ log_lldp_errno(lldp, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/ndisc-internal.h b/src/libsystemd-network/ndisc-internal.h index d7aa69a181..490305c329 100644 --- a/src/libsystemd-network/ndisc-internal.h +++ b/src/libsystemd-network/ndisc-internal.h @@ -41,15 +41,10 @@ struct sd_ndisc { const char* ndisc_event_to_string(sd_ndisc_event_t e) _const_; sd_ndisc_event_t ndisc_event_from_string(const char *s) _pure_; -#define log_ndisc_errno(ndisc, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_ndisc_get_ifname(ndisc), \ - LOG_DEBUG, _e, "NDISC: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) -#define log_ndisc(ndisc, fmt, ...) \ +#define log_ndisc_errno(ndisc, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "NDISC: ", \ + sd_ndisc_get_ifname(ndisc), \ + error, fmt, ##__VA_ARGS__) +#define log_ndisc(ndisc, fmt, ...) \ log_ndisc_errno(ndisc, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/radv-internal.h b/src/libsystemd-network/radv-internal.h index eef6db514a..7586ce6755 100644 --- a/src/libsystemd-network/radv-internal.h +++ b/src/libsystemd-network/radv-internal.h @@ -125,15 +125,10 @@ struct sd_radv_route_prefix { LIST_FIELDS(struct sd_radv_route_prefix, prefix); }; -#define log_radv_errno(radv, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_radv_get_ifname(radv), \ - LOG_DEBUG, _e, "RADV: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) -#define log_radv(radv, fmt, ...) \ +#define log_radv_errno(radv, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "RADV: ", \ + sd_radv_get_ifname(radv), \ + error, fmt, ##__VA_ARGS__) +#define log_radv(radv, fmt, ...) \ log_radv_errno(radv, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index 65a8588725..a2f3ab4921 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -75,16 +75,11 @@ struct sd_ipv4acd { void* userdata; }; -#define log_ipv4acd_errno(acd, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_ipv4acd_get_ifname(acd), \ - LOG_DEBUG, _e, "IPv4ACD: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) +#define log_ipv4acd_errno(acd, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "IPv4ACD: ", \ + sd_ipv4acd_get_ifname(acd), \ + error, fmt, ##__VA_ARGS__) #define log_ipv4acd(acd, fmt, ...) \ log_ipv4acd_errno(acd, 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c index cdde412a55..448449059e 100644 --- a/src/libsystemd-network/sd-ipv4ll.c +++ b/src/libsystemd-network/sd-ipv4ll.c @@ -49,16 +49,11 @@ struct sd_ipv4ll { void* userdata; }; -#define log_ipv4ll_errno(ll, error, fmt, ...) \ - ({ \ - int _e = (error); \ - if (DEBUG_LOGGING) \ - log_interface_full_errno_zerook( \ - sd_ipv4ll_get_ifname(ll), \ - LOG_DEBUG, _e, "IPv4LL: " fmt, \ - ##__VA_ARGS__); \ - -ERRNO_VALUE(_e); \ - }) +#define log_ipv4ll_errno(ll, error, fmt, ...) \ + log_interface_prefix_full_errno_zerook( \ + "IPv4LL: ", \ + sd_ipv4ll_get_ifname(ll), \ + error, fmt, ##__VA_ARGS__) #define log_ipv4ll(ll, fmt, ...) \ log_ipv4ll_errno(ll, 0, fmt, ##__VA_ARGS__) diff --git a/src/shared/log-link.h b/src/shared/log-link.h index 5f2b176353..20eab4cc04 100644 --- a/src/shared/log-link.h +++ b/src/shared/log-link.h @@ -17,6 +17,17 @@ log_interface_full_errno_zerook(ifname, level, _error, __VA_ARGS__); \ }) +#define log_interface_prefix_full_errno_zerook(prefix, ifname_expr, error, fmt, ...) \ + ({ \ + int _e = (error); \ + if (DEBUG_LOGGING) \ + log_interface_full_errno_zerook( \ + ifname_expr, \ + LOG_DEBUG, _e, prefix fmt, \ + ##__VA_ARGS__); \ + -ERRNO_VALUE(_e); \ + }) + /* * The following macros append INTERFACE= to the message. * The macros require a struct named 'Link' which contains 'char *ifname': From 00dd6d77702e5e8148cced2ed3d028cae9788919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 14 Apr 2021 23:30:27 +0200 Subject: [PATCH 15/20] libsystemd-network: check that errno==0 is not passed to log functions --- src/libsystemd-network/dhcp-internal.h | 7 +++++-- src/libsystemd-network/dhcp-server-internal.h | 7 +++++-- src/libsystemd-network/dhcp6-internal.h | 7 +++++-- src/libsystemd-network/lldp-internal.h | 7 +++++-- src/libsystemd-network/ndisc-internal.h | 7 +++++-- src/libsystemd-network/radv-internal.h | 7 +++++-- src/libsystemd-network/sd-ipv4acd.c | 7 +++++-- src/libsystemd-network/sd-ipv4ll.c | 7 +++++-- src/shared/log-link.h | 7 +++++++ 9 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/libsystemd-network/dhcp-internal.h b/src/libsystemd-network/dhcp-internal.h index ee2ce2b250..e5be7c5a63 100644 --- a/src/libsystemd-network/dhcp-internal.h +++ b/src/libsystemd-network/dhcp-internal.h @@ -67,9 +67,12 @@ int dhcp_packet_verify_headers(DHCPPacket *packet, size_t len, bool checksum, ui _cleanup_(sd_dhcp_client_unrefp) _unused_ sd_dhcp_client *_dont_destroy_##client = sd_dhcp_client_ref(client) #define log_dhcp_client_errno(client, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "DHCPv4 client: ", \ sd_dhcp_client_get_ifname(client), \ error, fmt, ##__VA_ARGS__) #define log_dhcp_client(client, fmt, ...) \ - log_dhcp_client_errno(client, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "DHCPv4 client: ", \ + sd_dhcp_client_get_ifname(client), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index 67c1cf4605..b905791f3c 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -99,9 +99,12 @@ void client_id_hash_func(const DHCPClientId *p, struct siphash *state); int client_id_compare_func(const DHCPClientId *a, const DHCPClientId *b); #define log_dhcp_server_errno(server, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "DHCPv4 server: ", \ sd_dhcp_server_get_ifname(server), \ error, fmt, ##__VA_ARGS__) #define log_dhcp_server(server, fmt, ...) \ - log_dhcp_server_errno(server, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "DHCPv4 server: ", \ + sd_dhcp_server_get_ifname(server), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 2b61a7bb3b..1e5a6b0e37 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -120,9 +120,12 @@ const char *dhcp6_message_status_to_string(int s) _const_; int dhcp6_message_status_from_string(const char *s) _pure_; #define log_dhcp6_client_errno(client, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "DHCPv6 client: ", \ sd_dhcp6_client_get_ifname(client), \ error, fmt, ##__VA_ARGS__) #define log_dhcp6_client(client, fmt, ...) \ - log_dhcp6_client_errno(client, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "DHCPv6 client: ", \ + sd_dhcp6_client_get_ifname(client), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/lldp-internal.h b/src/libsystemd-network/lldp-internal.h index 8846fa5323..cf0578c5c2 100644 --- a/src/libsystemd-network/lldp-internal.h +++ b/src/libsystemd-network/lldp-internal.h @@ -37,9 +37,12 @@ const char* lldp_event_to_string(sd_lldp_event_t e) _const_; sd_lldp_event_t lldp_event_from_string(const char *s) _pure_; #define log_lldp_errno(lldp, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "LLDP: ", \ sd_lldp_get_ifname(lldp), \ error, fmt, ##__VA_ARGS__) #define log_lldp(lldp, fmt, ...) \ - log_lldp_errno(lldp, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "LLDP: ", \ + sd_lldp_get_ifname(lldp), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/ndisc-internal.h b/src/libsystemd-network/ndisc-internal.h index 490305c329..d43b575352 100644 --- a/src/libsystemd-network/ndisc-internal.h +++ b/src/libsystemd-network/ndisc-internal.h @@ -42,9 +42,12 @@ const char* ndisc_event_to_string(sd_ndisc_event_t e) _const_; sd_ndisc_event_t ndisc_event_from_string(const char *s) _pure_; #define log_ndisc_errno(ndisc, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "NDISC: ", \ sd_ndisc_get_ifname(ndisc), \ error, fmt, ##__VA_ARGS__) #define log_ndisc(ndisc, fmt, ...) \ - log_ndisc_errno(ndisc, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "NDISC: ", \ + sd_ndisc_get_ifname(ndisc), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/radv-internal.h b/src/libsystemd-network/radv-internal.h index 7586ce6755..209425548b 100644 --- a/src/libsystemd-network/radv-internal.h +++ b/src/libsystemd-network/radv-internal.h @@ -126,9 +126,12 @@ struct sd_radv_route_prefix { }; #define log_radv_errno(radv, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "RADV: ", \ sd_radv_get_ifname(radv), \ error, fmt, ##__VA_ARGS__) #define log_radv(radv, fmt, ...) \ - log_radv_errno(radv, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "RADV: ", \ + sd_radv_get_ifname(radv), \ + 0, fmt, ##__VA_ARGS__) diff --git a/src/libsystemd-network/sd-ipv4acd.c b/src/libsystemd-network/sd-ipv4acd.c index a2f3ab4921..b4af60741a 100644 --- a/src/libsystemd-network/sd-ipv4acd.c +++ b/src/libsystemd-network/sd-ipv4acd.c @@ -76,12 +76,15 @@ struct sd_ipv4acd { }; #define log_ipv4acd_errno(acd, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "IPv4ACD: ", \ sd_ipv4acd_get_ifname(acd), \ error, fmt, ##__VA_ARGS__) #define log_ipv4acd(acd, fmt, ...) \ - log_ipv4acd_errno(acd, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "IPv4ACD: ", \ + sd_ipv4acd_get_ifname(acd), \ + 0, fmt, ##__VA_ARGS__) static const char * const ipv4acd_state_table[_IPV4ACD_STATE_MAX] = { [IPV4ACD_STATE_INIT] = "init", diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c index 448449059e..8053afee93 100644 --- a/src/libsystemd-network/sd-ipv4ll.c +++ b/src/libsystemd-network/sd-ipv4ll.c @@ -50,12 +50,15 @@ struct sd_ipv4ll { }; #define log_ipv4ll_errno(ll, error, fmt, ...) \ - log_interface_prefix_full_errno_zerook( \ + log_interface_prefix_full_errno( \ "IPv4LL: ", \ sd_ipv4ll_get_ifname(ll), \ error, fmt, ##__VA_ARGS__) #define log_ipv4ll(ll, fmt, ...) \ - log_ipv4ll_errno(ll, 0, fmt, ##__VA_ARGS__) + log_interface_prefix_full_errno_zerook( \ + "IPv4LL: ", \ + sd_ipv4ll_get_ifname(ll), \ + 0, fmt, ##__VA_ARGS__) static void ipv4ll_on_acd(sd_ipv4acd *ll, int event, void *userdata); diff --git a/src/shared/log-link.h b/src/shared/log-link.h index 20eab4cc04..51eaa0c06e 100644 --- a/src/shared/log-link.h +++ b/src/shared/log-link.h @@ -28,6 +28,13 @@ -ERRNO_VALUE(_e); \ }) +#define log_interface_prefix_full_errno(prefix, ifname_expr, error, fmt, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_interface_prefix_full_errno_zerook(prefix, ifname_expr, _error, fmt, ##__VA_ARGS__); \ + }) + /* * The following macros append INTERFACE= to the message. * The macros require a struct named 'Link' which contains 'char *ifname': From ec77d1ab3b9cc156f724515779e9ecb2aa8b4188 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Apr 2021 07:49:31 +0200 Subject: [PATCH 16/20] udev/cdrom_id: do not pass ioctl return value to log_debug_errno() While at it, let's print the tray status in human readable form. --- src/udev/cdrom_id/cdrom_id.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index 60c7b4922c..f1715a93bc 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -302,10 +302,23 @@ static int cd_capability_compat(Context *c) { } static int cd_media_compat(Context *c) { + int r; + assert(c); - if (ioctl(c->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) != CDS_DISC_OK) - return log_debug_errno(errno, "CDROM_DRIVE_STATUS != CDS_DISC_OK"); + r = ioctl(c->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT); + if (r < 0) + return log_debug_errno(errno, "ioctl(CDROM_DRIVE_STATUS) failed: m"); + if (r != CDS_DISC_OK) { + log_debug("ioctl(CDROM_DRIVE_STATUS) → %d (%s), ignoring", + r, + r == CDS_NO_INFO ? "no info" : + r == CDS_NO_DISC ? "no disc" : + r == CDS_TRAY_OPEN ? "tray open" : + r == CDS_DRIVE_NOT_READY ? "drive not ready" : + "unkown status"); + return -ENOMEDIUM; + } c->has_media = true; return 0; From 3d346b8106a0dad2bdc7f008bea80e65b05c1600 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Apr 2021 07:51:58 +0200 Subject: [PATCH 17/20] udev/cdrom_id: drop unnecessary cleanup and simplify loop --- src/udev/cdrom_id/cdrom_id.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/udev/cdrom_id/cdrom_id.c b/src/udev/cdrom_id/cdrom_id.c index f1715a93bc..2523752043 100644 --- a/src/udev/cdrom_id/cdrom_id.c +++ b/src/udev/cdrom_id/cdrom_id.c @@ -743,25 +743,23 @@ static int cd_media_toc(Context *c) { } static int open_drive(Context *c) { - _cleanup_close_ int fd = -1; + int fd; assert(c); assert(c->fd < 0); - for (int cnt = 0; cnt < 20; cnt++) { - if (cnt != 0) - (void) usleep(100 * USEC_PER_MSEC + random_u64() % (100 * USEC_PER_MSEC)); - + for (int cnt = 0;; cnt++) { fd = open(arg_node, O_RDONLY|O_NONBLOCK|O_CLOEXEC); - if (fd >= 0 || errno != EBUSY) + if (fd >= 0) break; + if (++cnt >= 20 || errno != EBUSY) + return log_debug_errno(errno, "Unable to open '%s': %m", arg_node); + + (void) usleep(100 * USEC_PER_MSEC + random_u64() % (100 * USEC_PER_MSEC)); } - if (fd < 0) - return log_debug_errno(errno, "Unable to open '%s'", arg_node); log_debug("probing: '%s'", arg_node); - - c->fd = TAKE_FD(fd); + c->fd = fd; return 0; } @@ -952,7 +950,7 @@ static int parse_argv(int argc, char *argv[]) { arg_node = argv[optind]; if (!arg_node) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No device is specified."); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No device specified."); return 1; } From 2669c66614cf10fe006863a0ad10484a26d3a51b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Apr 2021 11:18:32 +0200 Subject: [PATCH 18/20] core/selinux: fix wrong assertion when 0 is passed to log_debug https://github.com/systemd/systemd/pull/19317#issuecomment-820245680 --- src/core/selinux-access.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c index 5605c71053..cdb82dd894 100644 --- a/src/core/selinux-access.c +++ b/src/core/selinux-access.c @@ -273,8 +273,9 @@ int mac_selinux_generic_access_check( sd_bus_error_set(error, SD_BUS_ERROR_ACCESS_DENIED, "SELinux policy denies access."); } - log_debug_errno(r, "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s path=%s cmdline=%s: %m", - scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", path, cl); + log_full_errno_zerook(LOG_DEBUG, r, + "SELinux access check scon=%s tcon=%s tclass=%s perm=%s state=%s path=%s cmdline=%s: %m", + scon, fcon, tclass, permission, enforce ? "enforcing" : "permissive", path, cl); return enforce ? r : 0; } From 9d9fed9ef4567a779ac6f9ac153745ef3bc8882a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Apr 2021 11:22:22 +0200 Subject: [PATCH 19/20] Voidify log_{device,token,rule}_debug() See analogous change for log_debug() for discussion. --- src/libsystemd/sd-device/device-util.h | 2 +- src/udev/net/link-config.c | 14 +++++++++----- src/udev/udev-rules.c | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 026bdcd644..0585c57bc9 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -60,7 +60,7 @@ #define log_device_full(device, level, ...) (void) log_device_full_errno(device, level, 0, __VA_ARGS__) -#define log_device_debug(device, ...) log_device_full_errno(device, LOG_DEBUG, 0, __VA_ARGS__) +#define log_device_debug(device, ...) log_device_full(device, LOG_DEBUG, __VA_ARGS__) #define log_device_info(device, ...) log_device_full(device, LOG_INFO, __VA_ARGS__) #define log_device_notice(device, ...) log_device_full(device, LOG_NOTICE, __VA_ARGS__) #define log_device_warning(device, ...) log_device_full(device, LOG_WARNING, __VA_ARGS__) diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 31e5d0cd67..87afe8383a 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -364,9 +364,11 @@ static int get_mac(sd_device *device, MACAddressPolicy policy, struct ether_addr return r; switch (addr_type) { case NET_ADDR_SET: - return log_device_debug(device, "MAC on the device already set by userspace"); + log_device_debug(device, "MAC on the device already set by userspace"); + return 0; case NET_ADDR_STOLEN: - return log_device_debug(device, "MAC on the device already set based on another device"); + log_device_debug(device, "MAC on the device already set based on another device"); + return 0; case NET_ADDR_RANDOM: case NET_ADDR_PERM: break; @@ -375,9 +377,11 @@ static int get_mac(sd_device *device, MACAddressPolicy policy, struct ether_addr return 0; } - if (want_random == (addr_type == NET_ADDR_RANDOM)) - return log_device_debug(device, "MAC on the device already matches policy *%s*", - mac_address_policy_to_string(policy)); + if (want_random == (addr_type == NET_ADDR_RANDOM)) { + log_device_debug(device, "MAC on the device already matches policy *%s*", + mac_address_policy_to_string(policy)); + return 0; + } if (want_random) { log_device_debug(device, "Using random bytes to generate MAC"); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 57ede6a197..ee7404b9f6 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -198,7 +198,7 @@ struct UdevRules { #define log_rule_full(device, rules, level, ...) (void) log_rule_full_errno(device, rules, level, 0, __VA_ARGS__) -#define log_rule_debug(device, rules, ...) log_rule_full_errno(device, rules, LOG_DEBUG, 0, __VA_ARGS__) +#define log_rule_debug(device, rules, ...) log_rule_full(device, rules, LOG_DEBUG, __VA_ARGS__) #define log_rule_info(device, rules, ...) log_rule_full(device, rules, LOG_INFO, __VA_ARGS__) #define log_rule_notice(device, rules, ...) log_rule_full(device, rules, LOG_NOTICE, __VA_ARGS__) #define log_rule_warning(device, rules, ...) log_rule_full(device, rules, LOG_WARNING, __VA_ARGS__) @@ -213,7 +213,7 @@ struct UdevRules { #define log_token_full_errno(rules, level, error, ...) log_rule_full_errno(NULL, rules, level, error, __VA_ARGS__) #define log_token_full(rules, level, ...) (void) log_token_full_errno(rules, level, 0, __VA_ARGS__) -#define log_token_debug(rules, ...) log_token_full_errno(rules, LOG_DEBUG, 0, __VA_ARGS__) +#define log_token_debug(rules, ...) log_token_full(rules, LOG_DEBUG, __VA_ARGS__) #define log_token_info(rules, ...) log_token_full(rules, LOG_INFO, __VA_ARGS__) #define log_token_notice(rules, ...) log_token_full(rules, LOG_NOTICE, __VA_ARGS__) #define log_token_warning(rules, ...) log_token_full(rules, LOG_WARNING, __VA_ARGS__) From 8d1c9489ef373fc4d8de326b49086faa01185f32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 15 Apr 2021 11:56:22 +0200 Subject: [PATCH 20/20] Assert zero is not passed to log_{device,rule,token}_*_errno() --- src/libsystemd/sd-device/device-util.h | 15 +++++++++++---- src/udev/udev-rules.c | 22 ++++++++++++++++------ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/libsystemd/sd-device/device-util.h b/src/libsystemd/sd-device/device-util.h index 0585c57bc9..e2adfe8132 100644 --- a/src/libsystemd/sd-device/device-util.h +++ b/src/libsystemd/sd-device/device-util.h @@ -45,20 +45,27 @@ device; \ device = sd_device_enumerator_get_subsystem_next(enumerator)) -#define log_device_full_errno(device, level, error, ...) \ +#define log_device_full_errno_zerook(device, level, error, ...) \ ({ \ const char *_sysname = NULL; \ sd_device *_d = (device); \ - int _level = (level), _error = (error); \ + int _level = (level), _e = (error); \ \ if (_d && _unlikely_(log_get_max_level() >= LOG_PRI(_level))) \ (void) sd_device_get_sysname(_d, &_sysname); \ - log_object_internal(_level, _error, PROJECT_FILE, __LINE__, __func__, \ + log_object_internal(_level, _e, PROJECT_FILE, __LINE__, __func__, \ _sysname ? "DEVICE=" : NULL, _sysname, \ NULL, NULL, __VA_ARGS__); \ }) -#define log_device_full(device, level, ...) (void) log_device_full_errno(device, level, 0, __VA_ARGS__) +#define log_device_full_errno(device, level, error, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_device_full_errno_zerook(device, level, _error, __VA_ARGS__); \ + }) + +#define log_device_full(device, level, ...) (void) log_device_full_errno_zerook(device, level, 0, __VA_ARGS__) #define log_device_debug(device, ...) log_device_full(device, LOG_DEBUG, __VA_ARGS__) #define log_device_info(device, ...) log_device_full(device, LOG_INFO, __VA_ARGS__) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index ee7404b9f6..58e66bec84 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -184,19 +184,28 @@ struct UdevRules { /*** Logging helpers ***/ -#define log_rule_full_errno(device, rules, level, error, fmt, ...) \ +#define log_rule_full_errno_zerook(device, rules, level, error, fmt, ...) \ ({ \ UdevRules *_r = (rules); \ UdevRuleFile *_f = _r ? _r->current_file : NULL; \ UdevRuleLine *_l = _f ? _f->current_line : NULL; \ const char *_n = _f ? _f->filename : NULL; \ \ - log_device_full_errno(device, level, error, "%s:%u " fmt, \ - strna(_n), _l ? _l->line_number : 0, \ - ##__VA_ARGS__); \ + log_device_full_errno_zerook( \ + device, level, error, "%s:%u " fmt, \ + strna(_n), _l ? _l->line_number : 0, \ + ##__VA_ARGS__); \ }) -#define log_rule_full(device, rules, level, ...) (void) log_rule_full_errno(device, rules, level, 0, __VA_ARGS__) +#define log_rule_full_errno(device, rules, level, error, fmt, ...) \ + ({ \ + int _error = (error); \ + ASSERT_NON_ZERO(_error); \ + log_rule_full_errno_zerook( \ + device, rules, level, _error, fmt, ##__VA_ARGS__); \ + }) + +#define log_rule_full(device, rules, level, ...) (void) log_rule_full_errno_zerook(device, rules, level, 0, __VA_ARGS__) #define log_rule_debug(device, rules, ...) log_rule_full(device, rules, LOG_DEBUG, __VA_ARGS__) #define log_rule_info(device, rules, ...) log_rule_full(device, rules, LOG_INFO, __VA_ARGS__) @@ -210,8 +219,9 @@ struct UdevRules { #define log_rule_warning_errno(device, rules, error, ...) log_rule_full_errno(device, rules, LOG_WARNING, error, __VA_ARGS__) #define log_rule_error_errno(device, rules, error, ...) log_rule_full_errno(device, rules, LOG_ERR, error, __VA_ARGS__) +#define log_token_full_errno_zerook(rules, level, error, ...) log_rule_full_errno_zerook(NULL, rules, level, error, __VA_ARGS__) #define log_token_full_errno(rules, level, error, ...) log_rule_full_errno(NULL, rules, level, error, __VA_ARGS__) -#define log_token_full(rules, level, ...) (void) log_token_full_errno(rules, level, 0, __VA_ARGS__) +#define log_token_full(rules, level, ...) (void) log_token_full_errno_zerook(rules, level, 0, __VA_ARGS__) #define log_token_debug(rules, ...) log_token_full(rules, LOG_DEBUG, __VA_ARGS__) #define log_token_info(rules, ...) log_token_full(rules, LOG_INFO, __VA_ARGS__)