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>

Ævar Arnfjörð Bjarmason committed Jul 5, 2022 at 15:47 UTC 06f5f8940c0335f2a5b0a7bbd086115f4659eaa8
4 files changed +57 -6
builtin/repack.c
-2
@@ -727,7 +727,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
727 struct child_process cmd = CHILD_PROCESS_INIT;
728 struct string_list_item *item;
729 struct string_list names = STRING_LIST_INIT_DUP;
730 - struct string_list rollback = STRING_LIST_INIT_NODUP;
730 struct string_list existing_nonkept_packs = STRING_LIST_INIT_DUP;
731 struct string_list existing_kept_packs = STRING_LIST_INIT_DUP;
732 struct pack_geometry *geometry = NULL;
@@ -1117,7 +1116,6 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
1116 }
1117
1118 string_list_clear(&names, 0);
1120 - string_list_clear(&rollback, 0);
1119 string_list_clear(&existing_nonkept_packs, 0);
1120 string_list_clear(&existing_kept_packs, 0);
1121 clear_pack_geometry(geometry);
contrib/coccinelle/tests/unused.c
+27
@@ -53,3 +53,30 @@ void test_strbuf(void)
53 return;
54 strbuf_release(&sb8);
55 }
56 +
57 +void test_other(void)
58 +{
59 + struct string_list l = STRING_LIST_INIT_DUP;
60 + struct strbuf sb = STRBUF_INIT;
61 +
62 + string_list_clear(&l, 0);
63 + string_list_clear(&sb, 0);
64 +}
65 +
66 +void test_worktrees(void)
67 +{
68 + struct worktree **w1 = get_worktrees();
69 + struct worktree **w2 = get_worktrees();
70 + struct worktree **w3;
71 + struct worktree **w4;
72 +
73 + w3 = get_worktrees();
74 + w4 = get_worktrees();
75 +
76 + use_it(w4);
77 +
78 + free_worktrees(w1);
79 + free_worktrees(w2);
80 + free_worktrees(w3);
81 + free_worktrees(w4);
82 +}
contrib/coccinelle/tests/unused.res
+15
@@ -28,3 +28,18 @@ void test_strbuf(void)
28 if (when_strict())
29 return;
30 }
31 +
32 +void test_other(void)
33 +{
34 +}
35 +
36 +void test_worktrees(void)
37 +{
38 + struct worktree **w4;
39 +
40 + w4 = get_worktrees();
41 +
42 + use_it(w4);
43 +
44 + free_worktrees(w4);
45 +}
contrib/coccinelle/unused.cocci
+15 -4
@@ -4,10 +4,13 @@
4 @@
5 type T;
6 identifier I;
7 -constant INIT_MACRO =~ "^STRBUF_INIT$";
7 +// STRBUF_INIT, but also e.g. STRING_LIST_INIT_DUP (so no anchoring)
8 +constant INIT_MACRO =~ "_INIT";
9 identifier MALLOC1 =~ "^x?[mc]alloc$";
9 -identifier INIT_CALL1 =~ "^strbuf_init$";
10 -identifier REL1 =~ "^strbuf_(release|reset)$";
10 +identifier INIT_ASSIGN1 =~ "^get_worktrees$";
11 +identifier INIT_CALL1 =~ "^[a-z_]*_init$";
12 +identifier REL1 =~ "^[a-z_]*_(release|reset|clear|free)$";
13 +identifier REL2 =~ "^(release|clear|free)_[a-z_]*$";
14 @@
15
16 (
@@ -18,15 +21,23 @@ identifier REL1 =~ "^strbuf_(release|reset)$";
21 - T I = INIT_MACRO;
22 |
23 - T I = MALLOC1(...);
24 +|
25 +- T I = INIT_ASSIGN1(...);
26 )
27
28 <... when != \( I \| &I \)
29 (
30 - \( INIT_CALL1 \)( \( I \| &I \), ...);
31 |
32 +- I = \( INIT_ASSIGN1 \)(...);
33 +|
34 - I = MALLOC1(...);
35 )
36 ...>
37
31 -- \( REL1 \)( \( &I \| I \) );
38 +(
39 +- \( REL1 \| REL2 \)( \( I \| &I \), ...);
40 +|
41 +- \( REL1 \| REL2 \)( \( &I \| I \) );
42 +)
43 ... when != \( I \| &I \)