From 3bc3c734c63450bbcb1f8182e32a5dbc2eabe5b8 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 1 Oct 2020 16:11:43 +0200 Subject: [PATCH 1/7] coccinelle: drop the custom isomorphisms My former dumb me didn't read the documentation properly, so with the introduction of custom isomorphisms I caused two issues: 1) Masked all standard isomorphisms defined by Coccinelle 2) Replace the original issue with a completely new one --- coccinelle/run-coccinelle.sh | 3 +-- coccinelle/systemd-definitions.iso | 20 -------------------- 2 files changed, 1 insertion(+), 22 deletions(-) delete mode 100644 coccinelle/systemd-definitions.iso diff --git a/coccinelle/run-coccinelle.sh b/coccinelle/run-coccinelle.sh index be80a76a5f..1fdef5f63e 100755 --- a/coccinelle/run-coccinelle.sh +++ b/coccinelle/run-coccinelle.sh @@ -10,7 +10,6 @@ EXCLUDED_PATHS=( ) top="$(git rev-parse --show-toplevel)" -iso_defs="$top/coccinelle/systemd-definitions.iso" args= # Create an array from files tracked by git... @@ -37,7 +36,7 @@ for SCRIPT in ${@-$top/coccinelle/*.cocci} ; do TMPFILE=`mktemp` echo "+ spatch --sp-file $SCRIPT $args ..." parallel --halt now,fail=1 --keep-order --noswap --max-args=20 \ - spatch --iso-file $iso_defs --sp-file $SCRIPT $args ::: "${files[@]}" \ + spatch --sp-file $SCRIPT $args ::: "${files[@]}" \ 2>"$TMPFILE" || cat "$TMPFILE" echo -e "--x-- Processed $SCRIPT --x--\n" done diff --git a/coccinelle/systemd-definitions.iso b/coccinelle/systemd-definitions.iso deleted file mode 100644 index 92db763a29..0000000000 --- a/coccinelle/systemd-definitions.iso +++ /dev/null @@ -1,20 +0,0 @@ -/* Statement isomorphisms - replace explicit checks against NULL with a - * shorter variant, which relies on C's downgrade-to-bool feature. - * The expression metavariables should be declared as pointers, however, - * that doesn't work well with complex expressions like: - * if (UNIT(p)->default_dependencies != NULL) - */ - -Statement -@@ -expression X; -statement S; -@@ -if (X == NULL) S => if (!X) S - -Statement -@@ -expression X; -statement S; -@@ -if (X != NULL) S => if (X) S From 473de9b7086d4de122283f68e554ac4357369e34 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 1 Oct 2020 16:13:30 +0200 Subject: [PATCH 2/7] coccinelle: fix the equals-null transformation The original issue with this transformation was that we were replacing the whole if statement instead of just the expression inside. That caused the code to be weirdly formatted, as Coccinelle put a new block around each replaced if statement. This version replaces just the inner expression if it's in its incorrect form, otherwise it just accepts it (to avoid recursion). --- coccinelle/equals-null.cocci | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/coccinelle/equals-null.cocci b/coccinelle/equals-null.cocci index 957d828a83..3fce0f4caa 100644 --- a/coccinelle/equals-null.cocci +++ b/coccinelle/equals-null.cocci @@ -2,13 +2,28 @@ expression e; statement s; @@ -- if (e == NULL) -+ if (!e) -s +if ( +( +!e +| +- e == NULL ++ !e +) + ) + {...} +else s + @@ expression e; statement s; @@ -- if (e != NULL) -+ if (e) -s +if ( +( +e +| +- e != NULL ++ e +) + ) + {...} +else s From aad54dbc74f4b76c714e8191c23138298a6efa4f Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Fri, 2 Oct 2020 11:31:47 +0200 Subject: [PATCH 3/7] coccinelle: disable flags-set where it doesn't make sense --- coccinelle/flags-set.cocci | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/coccinelle/flags-set.cocci b/coccinelle/flags-set.cocci index 73966b02e5..f6cc8ba68a 100644 --- a/coccinelle/flags-set.cocci +++ b/coccinelle/flags-set.cocci @@ -1,7 +1,13 @@ @@ -/* Disable this transformation for the securebits-util.h, as it makes - * the expression there confusing. */ -position p : script:python() { p[0].file != "src/shared/securebits-util.h" }; +/* Disable this transformation in cases where it doesn't make sense or + * where it makes the resulting expression more confusing + */ +position p : script:python() { + not (p[0].file == "src/shared/securebits-util.h" or + p[0].file == "src/core/manager.h" or + p[0].current_element == "log_set_max_level_realm" or + p[0].current_element == "unichar_is_valid") + }; expression x, y; @@ ( From cb60571b31597441286d0dbde0ce179069bd45c5 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 4 Oct 2020 11:31:51 +0200 Subject: [PATCH 4/7] coccinelle: skip the xsprintf transformation on man pages since we don't expose xsprintf to users. --- coccinelle/xsprintf.cocci | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coccinelle/xsprintf.cocci b/coccinelle/xsprintf.cocci index 401216ad72..660a35e3f4 100644 --- a/coccinelle/xsprintf.cocci +++ b/coccinelle/xsprintf.cocci @@ -1,6 +1,7 @@ @@ +position p : script:python() { not p[0].file.startswith("man/") }; expression e, fmt; expression list vaargs; @@ -- snprintf(e, sizeof(e), fmt, vaargs); +- snprintf@p(e, sizeof(e), fmt, vaargs); + xsprintf(e, fmt, vaargs); From 1813613fedaf65d54bae8048cf9f922ec643fbd7 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 4 Oct 2020 11:44:48 +0200 Subject: [PATCH 5/7] coccinelle: limit the # of expressions in in_set/not_in_set transformations. Otherwise the time and resources to generate all possible permutations is simply unreasonable for running on local machines. --- coccinelle/in_set.cocci | 16 ++-------------- coccinelle/not_in_set.cocci | 15 ++------------- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/coccinelle/in_set.cocci b/coccinelle/in_set.cocci index 2c9b94ceb6..1c17c7df1f 100644 --- a/coccinelle/in_set.cocci +++ b/coccinelle/in_set.cocci @@ -1,25 +1,13 @@ +/* Limit the number of expressions to 6 for performance reasons */ @@ expression e; /* Exclude JsonVariant * from the transformation, as it can't work with the * current version of the IN_SET macro */ typedef JsonVariant; type T != JsonVariant*; -constant T n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; +constant T n0, n1, n2, n3, n4, n5; @@ - ( -- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 || e == n9 -+ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) -| -- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 || e == n8 -+ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) -| -- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 || e == n7 -+ IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) -| -- e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6 -+ IN_SET(e, n0, n1, n2, n3, n4, n5, n6) -| - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 + IN_SET(e, n0, n1, n2, n3, n4, n5) | diff --git a/coccinelle/not_in_set.cocci b/coccinelle/not_in_set.cocci index aed2c3490c..3486cff5df 100644 --- a/coccinelle/not_in_set.cocci +++ b/coccinelle/not_in_set.cocci @@ -1,22 +1,11 @@ +/* Limit the number of expressions to 6 for performance reasons */ @@ expression e; typedef JsonVariant; type T != JsonVariant*; -constant T n0, n1, n2, n3, n4, n5, n6, n7, n8, n9; +constant T n0, n1, n2, n3, n4, n5; @@ ( -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 && e != n9 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9) -| -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 && e != n8 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8) -| -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 && e != n7 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7) -| -- e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6 -+ !IN_SET(e, n0, n1, n2, n3, n4, n5, n6) -| - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 + !IN_SET(e, n0, n1, n2, n3, n4, n5) | From 135a9868a7551fba29a928f7a8dd9761486f9a5b Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 4 Oct 2020 12:29:43 +0200 Subject: [PATCH 6/7] coccinelle: skip strjoin transformation in test_strjoin() --- coccinelle/strjoin.cocci | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/coccinelle/strjoin.cocci b/coccinelle/strjoin.cocci index 675760e37a..46f70c4c23 100644 --- a/coccinelle/strjoin.cocci +++ b/coccinelle/strjoin.cocci @@ -1,16 +1,15 @@ @@ -expression list args; -@@ -- strjoin(args, NULL); -+ strjoin(args); -@@ +position p : script:python() { p[0].current_element != "test_strjoin" }; expression t; expression list args; @@ -- t = strjoin(args, NULL); +( +- strjoin@p(args, NULL); ++ strjoin(args); +| +- t = strjoin@p(args, NULL); + t = strjoin(args); -@@ -expression list args; -@@ -- return strjoin(args, NULL); +| +- return strjoin@p(args, NULL); + return strjoin(args); +) From 1d6cc5d0e5658848ac8cce5da22626d17f15b1ec Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Sun, 4 Oct 2020 11:29:23 +0200 Subject: [PATCH 7/7] tree-wide: coccinelle fixes --- src/basic/rm-rf.c | 2 +- src/core/unit.c | 4 ++-- src/libsystemd-network/sd-dhcp-lease.c | 2 +- src/libsystemd-network/sd-dhcp6-client.c | 2 +- src/shared/calendarspec.c | 2 +- src/shared/chown-recursive.c | 4 ++-- src/test/test-namespace.c | 4 ++-- src/udev/scsi_id/scsi_id.c | 2 +- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/basic/rm-rf.c b/src/basic/rm-rf.c index 01ff6bb331..ab331cd677 100644 --- a/src/basic/rm-rf.c +++ b/src/basic/rm-rf.c @@ -45,7 +45,7 @@ static int unlinkat_harder( return -errno; if (!S_ISDIR(st.st_mode)) return -ENOTDIR; - if ((st.st_mode & 0700) == 0700) /* Already set? */ + if (FLAGS_SET(st.st_mode, 0700)) /* Already set? */ return -EACCES; /* original error */ if (st.st_uid != geteuid()) /* this only works if the UID matches ours */ return -EACCES; diff --git a/src/core/unit.c b/src/core/unit.c index c3d7d24ecc..43373f35d4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5502,7 +5502,7 @@ void unit_remove_dependencies(Unit *u, UnitDependencyMask mask) { done = true; HASHMAP_FOREACH_KEY(di.data, other, u->dependencies[d]) { - if ((di.origin_mask & ~mask) == di.origin_mask) + if (FLAGS_SET(~mask, di.origin_mask)) continue; di.origin_mask &= ~mask; unit_update_dependency_mask(u, d, other, di); @@ -5516,7 +5516,7 @@ void unit_remove_dependencies(Unit *u, UnitDependencyMask mask) { UnitDependencyInfo dj; dj.data = hashmap_get(other->dependencies[q], u); - if ((dj.destination_mask & ~mask) == dj.destination_mask) + if (FLAGS_SET(~mask, dj.destination_mask)) continue; dj.destination_mask &= ~mask; diff --git a/src/libsystemd-network/sd-dhcp-lease.c b/src/libsystemd-network/sd-dhcp-lease.c index 0bc5fa3210..83e1b90291 100644 --- a/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/libsystemd-network/sd-dhcp-lease.c @@ -779,7 +779,7 @@ int dhcp_lease_parse_search_domains(const uint8_t *option, size_t len, char ***d return r; n += r; - } else if ((c & 0xc0) == 0xc0) { + } else if (FLAGS_SET(c, 0xc0)) { /* Pointer */ uint8_t d; diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 6bbc23f131..38025b63d9 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -270,7 +270,7 @@ static int dhcp6_client_set_duid_internal( assert_return(duid_len == 0 || duid != NULL, -EINVAL); assert_return(IN_SET(client->state, DHCP6_STATE_STOPPED), -EBUSY); - if (duid != NULL) { + if (duid) { r = dhcp_validate_duid_len(duid_type, duid_len, true); if (r < 0) { r = dhcp_validate_duid_len(duid_type, duid_len, false); diff --git a/src/shared/calendarspec.c b/src/shared/calendarspec.c index db6a103c42..bcc51f973c 100644 --- a/src/shared/calendarspec.c +++ b/src/shared/calendarspec.c @@ -363,7 +363,7 @@ int calendar_spec_to_string(const CalendarSpec *c, char **p) { if (c->utc) fputs(" UTC", f); - else if (c->timezone != NULL) { + else if (c->timezone) { fputc(' ', f); fputs(c->timezone, f); } else if (IN_SET(c->dst, 0, 1)) { diff --git a/src/shared/chown-recursive.c b/src/shared/chown-recursive.c index 636c0e2a7f..cb75d9a11b 100644 --- a/src/shared/chown-recursive.c +++ b/src/shared/chown-recursive.c @@ -124,7 +124,7 @@ int path_chown_recursive( if (fd < 0) return -errno; - if (!uid_is_valid(uid) && !gid_is_valid(gid) && (mask & 07777) == 07777) + if (!uid_is_valid(uid) && !gid_is_valid(gid) && FLAGS_SET(mask, 07777)) return 0; /* nothing to do */ if (fstat(fd, &st) < 0) @@ -160,7 +160,7 @@ int fd_chown_recursive( if (!S_ISDIR(st.st_mode)) return -ENOTDIR; - if (!uid_is_valid(uid) && !gid_is_valid(gid) && (mask & 07777) == 07777) + if (!uid_is_valid(uid) && !gid_is_valid(gid) && FLAGS_SET(mask, 07777)) return 0; /* nothing to do */ /* Shortcut, as above */ diff --git a/src/test/test-namespace.c b/src/test/test-namespace.c index 3972207329..7571e609a2 100644 --- a/src/test/test-namespace.c +++ b/src/test/test-namespace.c @@ -46,7 +46,7 @@ static void test_tmpdir(const char *id, const char *A, const char *B) { c = strjoina(a, "/tmp"); assert_se(stat(c, &x) >= 0); assert_se(S_ISDIR(x.st_mode)); - assert_se((x.st_mode & 01777) == 01777); + assert_se(FLAGS_SET(x.st_mode, 01777)); assert_se(rmdir(c) >= 0); assert_se(rmdir(a) >= 0); } @@ -57,7 +57,7 @@ static void test_tmpdir(const char *id, const char *A, const char *B) { d = strjoina(b, "/tmp"); assert_se(stat(d, &y) >= 0); assert_se(S_ISDIR(y.st_mode)); - assert_se((y.st_mode & 01777) == 01777); + assert_se(FLAGS_SET(y.st_mode, 01777)); assert_se(rmdir(d) >= 0); assert_se(rmdir(b) >= 0); } diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index bb08da28b5..9c33a560a8 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -236,7 +236,7 @@ static int get_file_options(const char *vendor, const char *model, retval = -1; break; } - if (vendor == NULL) { + if (!vendor) { if (!vendor_in) break; } else if (vendor_in &&