Merge pull request #12420 from mrc0mmand/coccinelle-tweaks

Coccinelle improvements

coccinelle/const-strlen.disabled

@@ -1,8 +1,12 @@
 @@
 constant s;
 @@
+(
+#define STRLEN
+&
 - sizeof(s)-1
 + STRLEN(s)
+)
 @@
 constant s;
 @@

coccinelle/debug-logging.cocci

@@ -1,8 +1,16 @@
 @@
 @@
+(
+#define DEBUG_LOGGING
+&
 - _unlikely_(log_get_max_level() >= LOG_DEBUG)
 + DEBUG_LOGGING
+)
 @@
 @@
+(
+#define DEBUG_LOGGING
+&
 - log_get_max_level() >= LOG_DEBUG
 + DEBUG_LOGGING
+)

coccinelle/dup-fcntl.cocci

@@ -1,5 +1,7 @@
 @@
+/* We want to stick with dup() in test-fd-util.c */
+position p : script:python() { p[0].file != "src/test/test-fd-util.c" };
 expression fd;
 @@
-- dup(fd)
+- dup@p(fd)
 + fcntl(fd, F_DUPFD, 3)

coccinelle/empty-to-null.cocci

@@ -1,5 +1,8 @@
 @@
+/* Avoid running this transformation on the empty_to_null function itself */
+position p : script:python() { p[0].current_element != "empty_to_null" };
 expression s;
 @@
-- isempty(s) ? NULL : s
+
+- isempty@p(s) ? NULL : s
 + empty_to_null(s)

coccinelle/flags-set.cocci

@@ -1,15 +1,16 @@
 @@
+/* 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" };
 expression x, y;
 @@
-- ((x) & (y)) == (y)
+(
+- ((x@p) & (y)) == (y)
 + FLAGS_SET(x, y)
-@@
-expression x, y;
-@@
-- (x & (y)) == (y)
+|
+- (x@p & (y)) == (y)
 + FLAGS_SET(x, y)
-@@
-expression x, y;
-@@
-- ((x) & y) == y
+|
+- ((x@p) & y) == y
 + FLAGS_SET(x, y)
+)

coccinelle/in_set.cocci

@@ -1,54 +1,37 @@
 @@
 expression e;
-constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9;
+/* 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;
 @@
+
+(
 - 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)
-@@
-expression e;
-constant 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 || e == n8
 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5, n6, n7;
-@@
+|
 - 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)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5, n6;
-@@
+|
 - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5 || e == n6
 + IN_SET(e, n0, n1, n2, n3, n4, n5, n6)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5;
-@@
+|
 - e == n0 || e == n1 || e == n2 || e == n3 || e == n4 || e == n5
 + IN_SET(e, n0, n1, n2, n3, n4, n5)
-@@
-expression e;
-constant n0, n1, n2, n3, n4;
-@@
+|
 - e == n0 || e == n1 || e == n2 || e == n3 || e == n4
 + IN_SET(e, n0, n1, n2, n3, n4)
-@@
-expression e;
-constant n0, n1, n2, n3;
-@@
+|
 - e == n0 || e == n1 || e == n2 || e == n3
 + IN_SET(e, n0, n1, n2, n3)
-@@
-expression e;
-constant n0, n1, n2;
-@@
+|
 - e == n0 || e == n1 || e == n2
 + IN_SET(e, n0, n1, n2)
-@@
-expression e;
-constant n0, n1;
-@@
+|
 - e == n0 || e == n1
 + IN_SET(e, n0, n1)
+)

coccinelle/isempty.cocci

@@ -1,60 +1,42 @@
 @@
+/* Disable this transformation for the test-string-util.c */
+position p : script:python() { p[0].file != "src/test/test-string-util.c" };
 expression s;
 @@
-- strv_length(s) == 0
+(
+- strv_length@p(s) == 0
 + strv_isempty(s)
-@@
-expression s;
-@@
-- strv_length(s) <= 0
+|
+- strv_length@p(s) <= 0
 + strv_isempty(s)
-@@
-expression s;
-@@
-- strv_length(s) > 0
+|
+- strv_length@p(s) > 0
 + !strv_isempty(s)
-@@
-expression s;
-@@
-- strv_length(s) != 0
+|
+- strv_length@p(s) != 0
 + !strv_isempty(s)
-@@
-expression s;
-@@
-- strlen(s) == 0
+|
+- strlen@p(s) == 0
 + isempty(s)
-@@
-expression s;
-@@
-- strlen(s) <= 0
+|
+- strlen@p(s) <= 0
 + isempty(s)
-@@
-expression s;
-@@
-- strlen(s) > 0
+|
+- strlen@p(s) > 0
 + !isempty(s)
-@@
-expression s;
-@@
-- strlen(s) != 0
+|
+- strlen@p(s) != 0
 + !isempty(s)
-@@
-expression s;
-@@
-- strlen_ptr(s) == 0
+|
+- strlen_ptr@p(s) == 0
 + isempty(s)
-@@
-expression s;
-@@
-- strlen_ptr(s) <= 0
+|
+- strlen_ptr@p(s) <= 0
 + isempty(s)
-@@
-expression s;
-@@
-- strlen_ptr(s) > 0
+|
+- strlen_ptr@p(s) > 0
 + !isempty(s)
-@@
-expression s;
-@@
-- strlen_ptr(s) != 0
+|
+- strlen_ptr@p(s) != 0
 + !isempty(s)
+)

coccinelle/memzero.cocci

@@ -21,10 +21,18 @@ expression s;
 @@
 expression a, b;
 @@
+(
+#define memzero
+&
 - memset(a, 0, b)
 + memzero(a, b)
+)
 @@
 expression a, b;
 @@
+(
+#define memzero
+&
 - bzero(a, b)
 + memzero(a, b)
+)

coccinelle/mfree_return.cocci

@@ -1,6 +1,8 @@
 @@
-expression p;
+/* Avoid running this transformation on the mfree function itself */
+position p : script:python() { p[0].current_element != "mfree" };
+expression e;
 @@
-- free(p);
+- free@p(e);
 - return NULL;
-+ return mfree(p);
++ return mfree(e);

coccinelle/not_in_set.cocci

@@ -1,54 +1,34 @@
 @@
 expression e;
-constant n0, n1, n2, n3, n4, n5, n6, n7, n8, n9;
+typedef JsonVariant;
+type T != JsonVariant*;
+constant T 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 && e != n9
 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8, n9)
-@@
-expression e;
-constant 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 && e != n8
 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6, n7, n8)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5, n6, n7;
-@@
+|
 - 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)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5, n6;
-@@
+|
 - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5 && e != n6
 + !IN_SET(e, n0, n1, n2, n3, n4, n5, n6)
-@@
-expression e;
-constant n0, n1, n2, n3, n4, n5;
-@@
+|
 - e != n0 && e != n1 && e != n2 && e != n3 && e != n4 && e != n5
 + !IN_SET(e, n0, n1, n2, n3, n4, n5)
-@@
-expression e;
-constant n0, n1, n2, n3, n4;
-@@
+|
 - e != n0 && e != n1 && e != n2 && e != n3 && e != n4
 + !IN_SET(e, n0, n1, n2, n3, n4)
-@@
-expression e;
-constant n0, n1, n2, n3;
-@@
+|
 - e != n0 && e != n1 && e != n2 && e != n3
 + !IN_SET(e, n0, n1, n2, n3)
-@@
-expression e;
-constant n0, n1, n2;
-@@
+|
 - e != n0 && e != n1 && e != n2
 + !IN_SET(e, n0, n1, n2)
-@@
-expression e;
-constant n0, n1;
-@@
+|
 - e != n0 && e != n1
 + !IN_SET(e, n0, n1)
+)

coccinelle/run-coccinelle.sh

@@ -1,9 +1,25 @@
 #!/bin/bash -e
 
+# Exclude following paths from the Coccinelle transformations
+EXCLUDED_PATHS=(
+    "src/boot/efi/*"
+    "src/shared/linux/*"
+    "src/basic/linux/*"
+    # Symlinked to test-bus-vtable-cc.cc, which causes issues with the IN_SET macro
+    "src/libsystemd/sd-bus/test-bus-vtable.c"
+)
+
 top="$(git rev-parse --show-toplevel)"
-files="$(git ls-files ':/*.[ch]')"
+iso_defs="$top/coccinelle/systemd-definitions.iso"
 args=
 
+# Create an array from files tracked by git...
+mapfile -t files < <(git ls-files ':/*.[ch]')
+# ...and filter everything that matches patterns from EXCLUDED_PATHS
+for excl in "${EXCLUDED_PATHS[@]}"; do
+    files=(${files[@]//$excl})
+done
+
 case "$1" in
     -i)
         args="$args --in-place"
@@ -21,7 +37,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 --sp-file $SCRIPT $args ::: $files \
+             spatch --iso-file $iso_defs --sp-file $SCRIPT $args ::: "${files[@]}" \
              2>"$TMPFILE" || cat "$TMPFILE"
     echo -e "--x-- Processed $SCRIPT --x--\n"
 done

coccinelle/strempty.cocci

@@ -1,48 +1,60 @@
 @@
+/* Avoid running this transformation on the strempty function itself */
+position p : script:python() { p[0].current_element != "strempty" };
 expression s;
 @@
-- s ?: ""
+(
+- s@p ?: ""
 + strempty(s)
-@@
-expression s;
-@@
-- s ? s : ""
+|
+- s@p ? s : ""
 + strempty(s)
+)
+
 @@
+position p : script:python() { p[0].current_element != "strempty" };
 expression s;
 @@
-- if (!s)
+- if (!s@p)
 -         s = "";
 + s = strempty(s);
+
 @@
+position p : script:python() { p[0].current_element != "strnull" };
 expression s;
 @@
-- s ?: "(null)"
+(
+- s@p ?: "(null)"
 + strnull(s)
-@@
-expression s;
-@@
-- s ? s : "(null)"
+|
+- s@p ? s : "(null)"
 + strnull(s)
+)
+
 @@
+position p : script:python() { p[0].current_element != "strnull" };
 expression s;
 @@
-- if (!s)
+- if (!s@p)
 -         s = "(null)";
 + s = strnull(s);
+
 @@
+position p : script:python() { p[0].current_element != "strna" };
 expression s;
 @@
-- s ?: "n/a"
+(
+- s@p ?: "n/a"
 + strna(s)
-@@
-expression s;
-@@
-- s ? s : "n/a"
+|
+- s@p ? s : "n/a"
 + strna(s)
+)
+
 @@
+position p : script:python() { p[0].current_element != "strna" };
 expression s;
 @@
-- if (!s)
+- if (!s@p)
 -         s = "n/a";
 + s = strna(s);

coccinelle/synthetic-errno.cocci

@@ -2,9 +2,15 @@
 expression e;
 expression list args;
 @@
+(
+/* Ignore one specific case in src/shared/bootspec.c where we want to stick
+ * with the log_debug() + return pattern */
+log_debug("Found no default boot entry :(");
+|
 - log_debug(args);
 - return -e;
 + return log_debug_errno(SYNTHETIC_ERRNO(e), args);
+)
 @@
 expression e;
 expression list args;

coccinelle/systemd-definitions.iso

@@ -0,0 +1,20 @@
+/* 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

coccinelle/take-fd.cocci

@@ -6,8 +6,15 @@ expression q;
 - q = -1;
 - return p;
 + return TAKE_FD(q);
+
+/* The ideal solution would use 'local idexpression' to avoid matching errno,
+ * which is a global variable. However, 'idexpression' nor 'identifier'
+ * would match, for example, "x->fd", which is considered 'expression' in
+ * the SmPL grammar
+ */
 @@
-expression p, q;
+expression p != errno;
+expression q;
 @@
 - p = q;
 - q = -1;

man/vtable-example.c

@@ -59,7 +59,7 @@ int main(int argc, char **argv) {
                                  vtable,
                                  &object));
 
-  while (true) {
+  for (;;) {
     check(sd_bus_wait(bus, UINT64_MAX));
     check(sd_bus_process(bus, NULL));
   }

src/core/dbus-socket.c

@@ -303,8 +303,12 @@ static int bus_socket_set_transient_property(
         if (streq(name, "SocketProtocol"))
                 return bus_set_transient_socket_protocol(u, name, &s->socket_protocol, message, flags, error);
 
-        if ((ci = socket_exec_command_from_string(name)) >= 0)
-                return bus_set_transient_exec_command(u, name, &s->exec_command[ci], message, flags, error);
+        ci = socket_exec_command_from_string(name);
+        if (ci >= 0)
+                return bus_set_transient_exec_command(u, name,
+                                                      &s->exec_command[ci],
+                                                      message, flags, error);
+
 
         if (streq(name, "Symlinks")) {
                 _cleanup_strv_free_ char **l = NULL;

src/coredump/coredump.c

@@ -1036,10 +1036,7 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd)
                                          * (truncated) copy of what we want to send, and the second one
                                          * contains the trailing dots. */
                                         copy[0] = iovec[i];
-                                        copy[1] = (struct iovec) {
-                                                .iov_base = (char[]) { '.', '.', '.' },
-                                                .iov_len = 3,
-                                        };
+                                        copy[1] = IOVEC_MAKE(((char[]){'.', '.', '.'}), 3);
 
                                         mh.msg_iov = copy;
                                         mh.msg_iovlen = 2;

src/import/importd.c

@@ -250,7 +250,7 @@ static void transfer_send_logs(Transfer *t, bool flush) {
                 n = strndup(t->log_message, e - t->log_message);
 
                 /* Skip over NUL and newlines */
-                while (e < t->log_message + t->log_message_size && (*e == 0 || *e == '\n'))
+                while (e < t->log_message + t->log_message_size && IN_SET(*e, 0, '\n'))
                         e++;
 
                 memmove(t->log_message, e, t->log_message + sizeof(t->log_message) - e);

src/journal-remote/journal-remote.c

@@ -181,7 +181,7 @@ static int get_source_for_fd(RemoteServer *s,
                 return log_warning_errno(r, "Failed to get writer for source %s: %m",
                                          name);
 
-        if (s->sources[fd] == NULL) {
+        if (!s->sources[fd]) {
                 s->sources[fd] = source_new(fd, false, name, writer);
                 if (!s->sources[fd]) {
                         writer_unref(writer);