cocci: generalize "unused" rule to cover more than "strbuf"

Generalize the newly added "unused.cocci" rule to find more than just
"struct strbuf", let's have it find the same unused patterns for
"struct string_list", as well as other code that uses
similar-looking *_{release,clear,free}() and {release,clear,free}_*()
functions.

We're intentionally loose in accepting e.g. a "strbuf_init(&sb)"
followed by a "string_list_clear(&sb, 0)".  It's assumed that the
compiler will catch any such invalid code, i.e. that our
constructors/destructors don't take a "void *".

See [1] for example of code that would be covered by the
"get_worktrees()" part of this rule. We'd still need work that the
series is based on (we were passing "worktrees" to a function), but
could now do the change in [1] automatically.

1. https://lore.kernel.org/git/Yq6eJFUPPTv%2Fzc0o@coredump.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/repack.c b/builtin/repack.c
index 4a7ae4cf489a4c34aaabeca064535f07c2df55f7..482b66f57d66b08f359c723fdf65808a34a05766 100644 (file)
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -727,7 +727,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        struct child_process cmd = CHILD_PROCESS_INIT;
        struct string_list_item *item;
        struct string_list names = STRING_LIST_INIT_DUP;
-       struct string_list rollback = STRING_LIST_INIT_NODUP;
        struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP;
        struct string_list existing_kept_packs = STRING_LIST_INIT_DUP;
        struct pack_geometry *geometry = NULL;
@@ -1117,7 +1116,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
        }
 
        string_list_clear(&names, 0);
-       string_list_clear(&rollback, 0);
        string_list_clear(&existing_nonkept_packs, 0);
        string_list_clear(&existing_kept_packs, 0);
        clear_pack_geometry(geometry);

diff --git a/contrib/coccinelle/tests/unused.c b/contrib/coccinelle/tests/unused.c
index 495ae58ccf1456b7a07c76d525156252a9f5060f..8294d734ba47d69def99f42d05d6069db0df26b1 100644 (file)
--- a/contrib/coccinelle/tests/unused.c
+++ b/contrib/coccinelle/tests/unused.c
@@ -53,3 +53,30 @@ void test_strbuf(void)
                return;
        strbuf_release(&sb8);
 }
+
+void test_other(void)
+{
+       struct string_list l = STRING_LIST_INIT_DUP;
+       struct strbuf sb = STRBUF_INIT;
+
+       string_list_clear(&l, 0);
+       string_list_clear(&sb, 0);
+}
+
+void test_worktrees(void)
+{
+       struct worktree **w1 = get_worktrees();
+       struct worktree **w2 = get_worktrees();
+       struct worktree **w3;
+       struct worktree **w4;
+
+       w3 = get_worktrees();
+       w4 = get_worktrees();
+
+       use_it(w4);
+
+       free_worktrees(w1);
+       free_worktrees(w2);
+       free_worktrees(w3);
+       free_worktrees(w4);
+}

diff --git a/contrib/coccinelle/tests/unused.res b/contrib/coccinelle/tests/unused.res
index b3b71053ed6ffc1145c108ff2e47036be4e1d66a..6d3e745683c8bcf2de5fbd84b0ca6452cc52857b 100644 (file)
--- a/contrib/coccinelle/tests/unused.res
+++ b/contrib/coccinelle/tests/unused.res
@@ -28,3 +28,18 @@ void test_strbuf(void)
        if (when_strict())
                return;
 }
+
+void test_other(void)
+{
+}
+
+void test_worktrees(void)
+{
+       struct worktree **w4;
+
+       w4 = get_worktrees();
+
+       use_it(w4);
+
+       free_worktrees(w4);
+}

diff --git a/contrib/coccinelle/unused.cocci b/contrib/coccinelle/unused.cocci
index 56530498d178b4df5602deeb8b09e87862a45d1a..d84046f82ea8f2c55b8a3d0ac2d112394703215d 100644 (file)
--- a/contrib/coccinelle/unused.cocci
+++ b/contrib/coccinelle/unused.cocci
@@ -4,10 +4,13 @@
 @@
 type T;
 identifier I;
-constant INIT_MACRO =~ "^STRBUF_INIT$";
+// STRBUF_INIT, but also e.g. STRING_LIST_INIT_DUP (so no anchoring)
+constant INIT_MACRO =~ "_INIT";
 identifier MALLOC1 =~ "^x?[mc]alloc$";
-identifier INIT_CALL1 =~ "^strbuf_init$";
-identifier REL1 =~ "^strbuf_(release|reset)$";
+identifier INIT_ASSIGN1 =~ "^get_worktrees$";
+identifier INIT_CALL1 =~ "^[a-z_]*_init$";
+identifier REL1 =~ "^[a-z_]*_(release|reset|clear|free)$";
+identifier REL2 =~ "^(release|clear|free)_[a-z_]*$";
 @@
 
 (
@@ -18,15 +21,23 @@ identifier REL1 =~ "^strbuf_(release|reset)$";
 - T I = INIT_MACRO;
 |
 - T I = MALLOC1(...);
+|
+- T I = INIT_ASSIGN1(...);
 )
 
 <... when != \( I \| &I \)
 (
 - \( INIT_CALL1 \)( \( I \| &I \), ...);
 |
+- I = \( INIT_ASSIGN1 \)(...);
+|
 - I = MALLOC1(...);
 )
 ...>
 
-- \( REL1 \)( \( &I \| I \) );
+(
+- \( REL1 \| REL2 \)( \( I \| &I \), ...);
+|
+- \( REL1 \| REL2 \)( \( &I \| I \) );
+)
   ... when != \( I \| &I \)