cocci: add and apply a rule to find "unused" strbufs
Add a coccinelle rule to remove "struct strbuf" initialization followed by calling "strbuf_release()" function, without any uses of the strbuf in the same function. See the tests in contrib/coccinelle/tests/unused.{c,res} for what it's intended to find and replace. The inclusion of "contrib/scalar/scalar.c" is because "spatch" was manually run on it (we don't usually run spatch on contrib). Per the "buggy code" comment we also match a strbuf_init() before the xmalloc(), but we're not seeking to be so strict as to make checks that the compiler will catch for us redundant. Saying we'll match either "init" or "xmalloc" lines makes the rule simpler. 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:46 UTC
4f40f6cb7365889b262aa93871964f70c91a9ebc
7 files changed
+119
-10
builtin/fetch.c
+1
-2
@@ -1113,7 +1113,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1113
struct fetch_head *fetch_head, struct worktree **worktrees)
1114
{
1115
int url_len, i, rc = 0;
1116
- struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
1116
+ struct strbuf note = STRBUF_INIT;
1117
const char *what, *kind;
1118
struct ref *rm;
1119
char *url;
@@ -1281,7 +1281,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
1281
1282
abort:
1283
strbuf_release(¬e);
1284
- strbuf_release(&err);
1284
free(url);
1285
return rc;
1286
}
builtin/merge.c
-4
@@ -375,7 +375,6 @@ static void reset_hard(const struct object_id *oid, int verbose)
375
static void restore_state(const struct object_id *head,
376
const struct object_id *stash)
377
{
378
- struct strbuf sb = STRBUF_INIT;
378
const char *args[] = { "stash", "apply", NULL, NULL };
379
380
if (is_null_oid(stash))
@@ -391,7 +390,6 @@ static void restore_state(const struct object_id *head,
390
*/
391
run_command_v_opt(args, RUN_GIT_CMD);
392
394
- strbuf_release(&sb);
393
refresh_cache(REFRESH_QUIET);
394
}
395
@@ -502,7 +500,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
500
{
501
struct commit *remote_head;
502
struct object_id branch_head;
505
- struct strbuf buf = STRBUF_INIT;
503
struct strbuf bname = STRBUF_INIT;
504
struct merge_remote_desc *desc;
505
const char *ptr;
@@ -590,7 +587,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
587
oid_to_hex(&remote_head->object.oid), remote);
588
cleanup:
589
free(found_ref);
593
- strbuf_release(&buf);
590
strbuf_release(&bname);
591
}
592
contrib/coccinelle/tests/unused.c
new
+55
@@ -0,0 +1,55 @@
1
+void test_strbuf(void)
2
+{
3
+ struct strbuf sb1 = STRBUF_INIT;
4
+ struct strbuf sb2 = STRBUF_INIT;
5
+ struct strbuf sb3 = STRBUF_INIT;
6
+ struct strbuf sb4 = STRBUF_INIT;
7
+ struct strbuf sb5;
8
+ struct strbuf sb6 = { 0 };
9
+ struct strbuf sb7 = STRBUF_INIT;
10
+ struct strbuf sb8 = STRBUF_INIT;
11
+ struct strbuf *sp1;
12
+ struct strbuf *sp2;
13
+ struct strbuf *sp3;
14
+ struct strbuf *sp4 = xmalloc(sizeof(struct strbuf));
15
+ struct strbuf *sp5 = xmalloc(sizeof(struct strbuf));
16
+ struct strbuf *sp6 = xmalloc(sizeof(struct strbuf));
17
+ struct strbuf *sp7;
18
+
19
+ strbuf_init(&sb5, 0);
20
+ strbuf_init(sp1, 0);
21
+ strbuf_init(sp2, 0);
22
+ strbuf_init(sp3, 0);
23
+ strbuf_init(sp4, 0);
24
+ strbuf_init(sp5, 0);
25
+ strbuf_init(sp6, 0);
26
+ strbuf_init(sp7, 0);
27
+ sp7 = xmalloc(sizeof(struct strbuf));
28
+
29
+ use_before(&sb3);
30
+ use_as_str("%s", sb7.buf);
31
+ use_as_str("%s", sp1->buf);
32
+ use_as_str("%s", sp6->buf);
33
+ pass_pp(&sp3);
34
+
35
+ strbuf_release(&sb1);
36
+ strbuf_reset(&sb2);
37
+ strbuf_release(&sb3);
38
+ strbuf_release(&sb4);
39
+ strbuf_release(&sb5);
40
+ strbuf_release(&sb6);
41
+ strbuf_release(&sb7);
42
+ strbuf_release(sp1);
43
+ strbuf_release(sp2);
44
+ strbuf_release(sp3);
45
+ strbuf_release(sp4);
46
+ strbuf_release(sp5);
47
+ strbuf_release(sp6);
48
+ strbuf_release(sp7);
49
+
50
+ use_after(&sb4);
51
+
52
+ if (when_strict())
53
+ return;
54
+ strbuf_release(&sb8);
55
+}
contrib/coccinelle/tests/unused.res
new
+30
@@ -0,0 +1,30 @@
1
+void test_strbuf(void)
2
+{
3
+ struct strbuf sb3 = STRBUF_INIT;
4
+ struct strbuf sb4 = STRBUF_INIT;
5
+ struct strbuf sb7 = STRBUF_INIT;
6
+ struct strbuf *sp1;
7
+ struct strbuf *sp3;
8
+ struct strbuf *sp6 = xmalloc(sizeof(struct strbuf));
9
+ strbuf_init(sp1, 0);
10
+ strbuf_init(sp3, 0);
11
+ strbuf_init(sp6, 0);
12
+
13
+ use_before(&sb3);
14
+ use_as_str("%s", sb7.buf);
15
+ use_as_str("%s", sp1->buf);
16
+ use_as_str("%s", sp6->buf);
17
+ pass_pp(&sp3);
18
+
19
+ strbuf_release(&sb3);
20
+ strbuf_release(&sb4);
21
+ strbuf_release(&sb7);
22
+ strbuf_release(sp1);
23
+ strbuf_release(sp3);
24
+ strbuf_release(sp6);
25
+
26
+ use_after(&sb4);
27
+
28
+ if (when_strict())
29
+ return;
30
+}
contrib/coccinelle/unused.cocci
new
+32
@@ -0,0 +1,32 @@
1
+// This rule finds sequences of "unused" declerations and uses of a
2
+// variable, where "unused" is defined to include only calling the
3
+// equivalent of alloc, init & free functions on the variable.
4
+@@
5
+type T;
6
+identifier I;
7
+constant INIT_MACRO =~ "^STRBUF_INIT$";
8
+identifier MALLOC1 =~ "^x?[mc]alloc$";
9
+identifier INIT_CALL1 =~ "^strbuf_init$";
10
+identifier REL1 =~ "^strbuf_(release|reset)$";
11
+@@
12
+
13
+(
14
+- T I;
15
+|
16
+- T I = { 0 };
17
+|
18
+- T I = INIT_MACRO;
19
+|
20
+- T I = MALLOC1(...);
21
+)
22
+
23
+<... when != \( I \| &I \)
24
+(
25
+- \( INIT_CALL1 \)( \( I \| &I \), ...);
26
+|
27
+- I = MALLOC1(...);
28
+)
29
+...>
30
+
31
+- \( REL1 \)( \( &I \| I \) );
32
+ ... when != \( I \| &I \)
contrib/scalar/scalar.c
+1
-2
@@ -687,7 +687,7 @@ static int cmd_diagnose(int argc, const char **argv)
687
int stdout_fd = -1, archiver_fd = -1;
688
time_t now = time(NULL);
689
struct tm tm;
690
- struct strbuf path = STRBUF_INIT, buf = STRBUF_INIT;
690
+ struct strbuf buf = STRBUF_INIT;
691
int res = 0;
692
693
argc = parse_options(argc, argv, NULL, options,
@@ -779,7 +779,6 @@ diagnose_cleanup:
779
free(argv_copy);
780
strvec_clear(&archiver_args);
781
strbuf_release(&zip_path);
782
- strbuf_release(&path);
782
strbuf_release(&buf);
783
784
return res;
diff.c
-2
@@ -1289,7 +1289,6 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
1289
{
1290
static const char *nneof = " No newline at end of file\n";
1291
const char *context, *reset, *set, *set_sign, *meta, *fraginfo;
1292
- struct strbuf sb = STRBUF_INIT;
1292
1293
enum diff_symbol s = eds->s;
1294
const char *line = eds->line;
@@ -1521,7 +1520,6 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
1520
default:
1521
BUG("unknown diff symbol");
1522
}
1524
- strbuf_release(&sb);
1523
}
1524
1525
static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,