diff --git a/coccinelle/log-json.cocci b/coccinelle/log-json.cocci index d184e5658454e..c941706c64171 100644 --- a/coccinelle/log-json.cocci +++ b/coccinelle/log-json.cocci @@ -3,7 +3,6 @@ expression e, v, flags; expression list args; @@ -+ return - json_log(v, flags, 0, args); -+ json_log(v, flags, SYNTHETIC_ERRNO(e), args); - return -e; ++ return json_log(v, flags, SYNTHETIC_ERRNO(e), args); diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index cd951790b9ddc..bb72a493f08f3 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -2,6 +2,14 @@ # SPDX-License-Identifier: LGPL-2.1-or-later set -e +# FIXME: +# - Coccinelle doesn't like our TEST() macros, which then causes name conflicts; i.e. Cocci can't process +# that TEST(xsetxattr) yields test_xsetxattr() and uses just xsetxattr() in this case, which then conflicts +# with the tested xsetxattr() function, leading up to the whole test case getting skipped due to +# conflicting typedefs +# - something keeps pulling in src/boot/efi/*.h stuff, even though it's excluded +# - Coccinelle has issues with some of our more complex macros + # Exclude following paths from the Coccinelle transformations EXCLUDED_PATHS=( "src/boot/efi/*" @@ -10,13 +18,17 @@ EXCLUDED_PATHS=( # Symlinked to test-bus-vtable-cc.cc, which causes issues with the IN_SET macro "src/libsystemd/sd-bus/test-bus-vtable.c" "src/libsystemd/sd-journal/lookup3.c" + # Ignore man examples, as they redefine some macros we use internally, which makes Coccinelle complain + # and ignore code that tries to use the redefined stuff + "man/*" ) TOP_DIR="$(git rev-parse --show-toplevel)" +CACHE_DIR="$(dirname "$0")/.coccinelle-cache" ARGS=() # Create an array from files tracked by git... -mapfile -t FILES < <(git ls-files ':/*.[ch]') +mapfile -t FILES < <(git ls-files ':/*.c') # ...and filter everything that matches patterns from EXCLUDED_PATHS for excl in "${EXCLUDED_PATHS[@]}"; do # shellcheck disable=SC2206 @@ -37,12 +49,43 @@ fi [[ ${#@} -ne 0 ]] && SCRIPTS=("$@") || SCRIPTS=("$TOP_DIR"/coccinelle/*.cocci) +mkdir -p "$CACHE_DIR" +echo "--x-- Using Coccinelle cache directory: $CACHE_DIR" +echo "--x--" +echo "--x-- Note: running spatch for the first time without populated cache takes" +echo "--x-- a _long_ time (15-30 minutes). Also, the cache is quite large" +echo "--x-- (~15 GiB), so make sure you have enough free space." +echo + for script in "${SCRIPTS[@]}"; do echo "--x-- Processing $script --x--" TMPFILE="$(mktemp)" echo "+ spatch --sp-file $script ${ARGS[*]} ..." - parallel --halt now,fail=1 --keep-order --noswap --max-args=20 \ - spatch --macro-file="$TOP_DIR/coccinelle/macros.h" --smpl-spacing --sp-file "$script" "${ARGS[@]}" ::: "${FILES[@]}" \ - 2>"$TMPFILE" || cat "$TMPFILE" + # A couple of notes: + # + # 1) Limit this to 10 files at once, as processing the ASTs is _very_ memory hungry - e.g. with 20 files + # at once one spatch process can take around 2.5 GiB of RAM, which can easily eat up all available RAM + # when paired together with parallel + # + # 2) Make sure spatch can find our includes via -I , similarly as we do when compiling stuff + # + # 3) Make sure to include includes from includes (--recursive-includes), but use them only to get type + # definitions (--include-headers-for-types) - otherwise we'd start formating them as well, which might be + # unwanted, especially for includes we fetch verbatim from third-parties + # + # 4) Use cache, since generating the full AST is _very_ expensive, i.e. the uncached run takes 15 - 30 + # minutes (for one rule(!)), vs 30 - 90 seconds when the cache is populated. One major downside of the + # cache is that it's quite big - ATTOW the cache takes around 15 GiB, but the performance boost is + # definitely worth it + parallel --halt now,fail=1 --keep-order --noswap --max-args=10 \ + spatch --cache-prefix "$CACHE_DIR" \ + -I src \ + --recursive-includes \ + --include-headers-for-types \ + --smpl-spacing \ + --sp-file "$script" \ + "${ARGS[@]}" ::: "${FILES[@]}" \ + 2>"$TMPFILE" || cat "$TMPFILE" + rm -f "$TMPFILE" echo -e "--x-- Processed $script --x--\n" done diff --git a/coccinelle/zz-drop-braces.cocci b/coccinelle/zz-drop-braces.cocci index 8c3be01c1f786..7a3382c9a7b9e 100644 --- a/coccinelle/zz-drop-braces.cocci +++ b/coccinelle/zz-drop-braces.cocci @@ -1,28 +1,13 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ @@ position p : script:python() { p[0].file != "src/journal/lookup3.c" }; -identifier id; -expression e; +expression e,e1; @@ -if (...) -- { +- if (e) { ++ if (e) ( - id@p(...); + e1@p; | - e@p; -) -- } - -@@ -position p : script:python() { p[0].file != "src/journal/lookup3.c" }; -identifier id; -expression e; -@@ -if (...) -- { -( - return id@p(...); -| - return e@p; + return e1@p; ) - } diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index 2c6dce0a0882b..1830d697848bb 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1311,7 +1311,7 @@ int bus_set_transient_exec_command( int r; /* Drop Ex from the written setting. E.g. ExecStart=, not ExecStartEx=. */ - const char *written_name = is_ex_prop ? strndupa(name, strlen(name) - 2) : name; + const char *written_name = is_ex_prop ? strndupa_safe(name, strlen(name) - 2) : name; r = sd_bus_message_enter_container(message, 'a', is_ex_prop ? "(sasas)" : "(sasb)"); if (r < 0) diff --git a/src/journal-remote/journal-gatewayd.c b/src/journal-remote/journal-gatewayd.c index ad1c46ac6ca27..d88436018c7b0 100644 --- a/src/journal-remote/journal-gatewayd.c +++ b/src/journal-remote/journal-gatewayd.c @@ -879,7 +879,7 @@ static int request_handler_machine( SD_ID128_FORMAT_VAL(bid), hostname_cleanup(hostname), os_release_pretty_name(pretty_name, os_name), - v ? v : "bare", + v ?: "bare", usage, cutoff_from, cutoff_to); diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index ff0228081fedb..5ac90223ffd88 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -627,7 +627,7 @@ static int message_new_reply( return r; } - t->dont_send = !!(call->header->flags & BUS_MESSAGE_NO_REPLY_EXPECTED); + t->dont_send = FLAGS_SET(call->header->flags, BUS_MESSAGE_NO_REPLY_EXPECTED); t->enforced_reply_signature = call->enforced_reply_signature; /* let's copy the sensitive flag over. Let's do that as a safety precaution to keep a transaction diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 3c59d0d615287..718709f0b24bd 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -217,7 +217,7 @@ static int bus_socket_auth_verify_client(sd_bus *b) { /* And possibly check the third line, too */ if (b->accept_fd) { l = lines[i++]; - b->can_fds = !!memory_startswith(l, lines[i] - l, "AGREE_UNIX_FD"); + b->can_fds = memory_startswith(l, lines[i] - l, "AGREE_UNIX_FD"); } assert(i == n); diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 4a0259f8bbd27..f036a49c644b5 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -321,7 +321,7 @@ _public_ int sd_bus_set_bus_client(sd_bus *bus, int b) { assert_return(!bus->patch_sender, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->bus_client = !!b; + bus->bus_client = b; return 0; } @@ -331,7 +331,7 @@ _public_ int sd_bus_set_monitor(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->is_monitor = !!b; + bus->is_monitor = b; return 0; } @@ -341,7 +341,7 @@ _public_ int sd_bus_negotiate_fds(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->accept_fd = !!b; + bus->accept_fd = b; return 0; } @@ -353,7 +353,7 @@ _public_ int sd_bus_negotiate_timestamp(sd_bus *bus, int b) { /* This is not actually supported by any of our transports these days, but we do honour it for synthetic * replies, and maybe one day classic D-Bus learns this too */ - bus->attach_timestamp = !!b; + bus->attach_timestamp = b; return 0; } @@ -380,7 +380,7 @@ _public_ int sd_bus_set_server(sd_bus *bus, int b, sd_id128_t server_id) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->is_server = !!b; + bus->is_server = b; bus->server_id = server_id; return 0; } @@ -391,7 +391,7 @@ _public_ int sd_bus_set_anonymous(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->anonymous_auth = !!b; + bus->anonymous_auth = b; return 0; } @@ -401,7 +401,7 @@ _public_ int sd_bus_set_trusted(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->trusted = !!b; + bus->trusted = b; return 0; } @@ -419,7 +419,7 @@ _public_ int sd_bus_set_allow_interactive_authorization(sd_bus *bus, int b) { assert_return(bus = bus_resolve(bus), -ENOPKG); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->allow_interactive_authorization = !!b; + bus->allow_interactive_authorization = b; return 0; } @@ -437,7 +437,7 @@ _public_ int sd_bus_set_watch_bind(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->watch_bind = !!b; + bus->watch_bind = b; return 0; } @@ -455,7 +455,7 @@ _public_ int sd_bus_set_connected_signal(sd_bus *bus, int b) { assert_return(bus->state == BUS_UNSET, -EPERM); assert_return(!bus_origin_changed(bus), -ECHILD); - bus->connected_signal = !!b; + bus->connected_signal = b; return 0; } diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c index ccdd0d50b7c03..2847ba84f5b13 100644 --- a/src/libsystemd/sd-bus/test-bus-objects.c +++ b/src/libsystemd/sd-bus/test-bus-objects.c @@ -494,10 +494,9 @@ static int client(struct context *c) { } assert_se(sd_bus_message_exit_container(reply) >= 0); - if (streq(path, "/value/a")) { + if (streq(path, "/value/a")) /* ObjectManager must be here */ assert_se(found_object_manager_interface); - } } else assert_se(sd_bus_message_skip(reply, "a{sa{sv}}") >= 0); diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c index 896ea4deca840..56f9ac7fc1d98 100644 --- a/src/libsystemd/sd-event/sd-event.c +++ b/src/libsystemd/sd-event/sd-event.c @@ -5038,7 +5038,7 @@ _public_ int sd_event_set_watchdog(sd_event *e, int b) { } } - e->watchdog = !!b; + e->watchdog = b; return e->watchdog; fail: diff --git a/src/modules-load/modules-load.c b/src/modules-load/modules-load.c index efca2379eac71..da7e3d8900555 100644 --- a/src/modules-load/modules-load.c +++ b/src/modules-load/modules-load.c @@ -172,10 +172,8 @@ static int run(int argc, char *argv[]) { log_warning_errno(r, "Failed to parse kernel command line, ignoring: %m"); ctx = kmod_new(NULL, NULL); - if (!ctx) { - log_error("Failed to allocate memory for kmod."); - return -ENOMEM; - } + if (!ctx) + return log_oom(); kmod_load_resources(ctx); kmod_set_log_fn(ctx, systemd_kmod_log, NULL); diff --git a/src/network/networkd-ndisc.c b/src/network/networkd-ndisc.c index fde5e2c3c1e6b..ddd60a1e57491 100644 --- a/src/network/networkd-ndisc.c +++ b/src/network/networkd-ndisc.c @@ -608,7 +608,7 @@ static int ndisc_router_process_route(Link *link, sd_ndisc_router *rt) { } r = sd_ndisc_router_route_get_preference(rt, &preference); - if (r == -ENOTSUP) { + if (r == -EOPNOTSUPP) { log_link_debug_errno(link, r, "Received route prefix with unsupported preference, ignoring: %m"); return 0; } diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index ac7cd43491768..0743a3427e7f0 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -2869,7 +2869,7 @@ int config_parse_route_tcp_rto( return 0; } - if (IN_SET(usec, 0, USEC_INFINITY) || + if (!timestamp_is_set(usec) || DIV_ROUND_UP(usec, USEC_PER_MSEC) > UINT32_MAX) { log_syntax(unit, LOG_WARNING, filename, line, 0, "Route TCP retransmission timeout (RTO) must be in the range 0…%"PRIu32"ms, ignoring assignment: %s", UINT32_MAX, rvalue); diff --git a/src/nspawn/nspawn-oci.c b/src/nspawn/nspawn-oci.c index e3b6f895d171d..62cf331a4a9c1 100644 --- a/src/nspawn/nspawn-oci.c +++ b/src/nspawn/nspawn-oci.c @@ -1837,10 +1837,8 @@ static int oci_seccomp_syscalls(const char *name, JsonVariant *v, JsonDispatchFl if (r < 0) return r; - if (strv_isempty(rule.names)) { - json_log(e, flags, 0, "System call name list is empty."); - return -EINVAL; - } + if (strv_isempty(rule.names)) + return json_log(e, flags, SYNTHETIC_ERRNO(EINVAL), "System call name list is empty."); STRV_FOREACH(i, rule.names) { int nr; diff --git a/src/partition/repart.c b/src/partition/repart.c index 1e9284e2e2e85..95cae94a8e069 100644 --- a/src/partition/repart.c +++ b/src/partition/repart.c @@ -4172,8 +4172,7 @@ static int sign_verity_roothash( return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to convert PKCS7 signature to DER: %s", ERR_error_string(ERR_get_error(), NULL)); - ret_signature->iov_base = TAKE_PTR(sig); - ret_signature->iov_len = sigsz; + *ret_signature = IOVEC_MAKE(TAKE_PTR(sig), sigsz); return 0; #else diff --git a/src/shared/killall.c b/src/shared/killall.c index 330b4c3272d77..917b7732665ff 100644 --- a/src/shared/killall.c +++ b/src/shared/killall.c @@ -257,7 +257,7 @@ static int killall(int sig, Set *pids, bool send_sighup) { r = pidref_kill(&pidref, sig); if (r < 0) { - if (errno != -ESRCH) + if (r != -ESRCH) log_warning_errno(errno, "Could not kill " PID_FMT ", ignoring: %m", pidref.pid); } else { n_killed++; diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index f2bdae2becac1..2a75db572dd5d 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -2059,7 +2059,7 @@ int tpm2_create_primary( session ? session->esys_handle : ESYS_TR_PASSWORD, ESYS_TR_NONE, ESYS_TR_NONE, - sensitive ? sensitive : &(TPM2B_SENSITIVE_CREATE) {}, + sensitive ?: &(TPM2B_SENSITIVE_CREATE) {}, template, /* outsideInfo= */ NULL, &(TPML_PCR_SELECTION) {}, @@ -5891,10 +5891,7 @@ int tpm2_unseal_data( "Failed to unseal data: %s", sym_Tss2_RC_Decode(rc)); _cleanup_(iovec_done) struct iovec d = {}; - d = (struct iovec) { - .iov_base = memdup(unsealed->buffer, unsealed->size), - .iov_len = unsealed->size, - }; + d = IOVEC_MAKE(memdup(unsealed->buffer, unsealed->size), unsealed->size); explicit_bzero_safe(unsealed->buffer, unsealed->size); diff --git a/src/systemctl/systemctl-edit.c b/src/systemctl/systemctl-edit.c index a6bb5e5289225..c851c8546ef55 100644 --- a/src/systemctl/systemctl-edit.c +++ b/src/systemctl/systemctl-edit.c @@ -247,13 +247,12 @@ static int find_paths_to_edit( return r; /* Already logged by unit_find_paths() */ if (!path) { - if (!arg_force) { - log_info("Run 'systemctl edit%s --force --full %s' to create a new unit.", - arg_runtime_scope == RUNTIME_SCOPE_GLOBAL ? " --global" : - arg_runtime_scope == RUNTIME_SCOPE_USER ? " --user" : "", - *name); - return -ENOENT; - } + if (!arg_force) + return log_info_errno(SYNTHETIC_ERRNO(ENOENT), + "Run 'systemctl edit%s --force --full %s' to create a new unit.", + arg_runtime_scope == RUNTIME_SCOPE_GLOBAL ? " --global" : + arg_runtime_scope == RUNTIME_SCOPE_USER ? " --user" : "", + *name); /* Create a new unit from scratch */ r = unit_file_create_new( diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c index eb9678a7d9478..331faf1e4b304 100644 --- a/src/test/test-acl-util.c +++ b/src/test/test-acl-util.c @@ -82,7 +82,7 @@ TEST(fd_acl_make_read_only) { (void) fd_add_uid_acl_permission(fd, 1, ACL_READ|ACL_WRITE|ACL_EXECUTE); assert_se(fstat(fd, &st) >= 0); - assert_se((st.st_mode & 0200) == 0200); + assert_se(FLAGS_SET(st.st_mode, 0200)); cmd = strjoina("getfacl -p ", fn); assert_se(system(cmd) == 0); diff --git a/src/test/test-uid-range.c b/src/test/test-uid-range.c index 186f6ee29c8d8..aabbd2425ccb4 100644 --- a/src/test/test-uid-range.c +++ b/src/test/test-uid-range.c @@ -99,7 +99,7 @@ TEST(load_userns) { int r; r = uid_range_load_userns(&p, NULL); - if (r < 0 && ERRNO_IS_NOT_SUPPORTED(r)) + if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) return; assert_se(r >= 0); diff --git a/src/udev/udev-builtin-path_id.c b/src/udev/udev-builtin-path_id.c index 467c9a6ad3981..f1370e60608ce 100644 --- a/src/udev/udev-builtin-path_id.c +++ b/src/udev/udev-builtin-path_id.c @@ -632,7 +632,7 @@ static int find_real_nvme_parent(sd_device *dev, sd_device **ret) { return -ENXIO; end += strspn(end, DIGITS); - sysname = strndupa(sysname, end - sysname); + sysname = strndupa_safe(sysname, end - sysname); r = sd_device_new_from_subsystem_sysname(&nvme, "nvme", sysname); if (r < 0)