git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 5/6] cocci: add and apply a rule to find "unused" strbufs

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 5, 2022, 13:46 UTC
Message-ID
<patch-v4-5.6-d1c6833c8d5-20220705T134033Z-avarab@gmail.com>
In-Reply-To
<cover-v4-0.6-00000000000-20220705T134033Z-avarab@gmail.com>

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>
---
 builtin/fetch.c                     |  3 +-
 builtin/merge.c                     |  4 ---
 contrib/coccinelle/tests/unused.c   | 55 +++++++++++++++++++++++++++++
 contrib/coccinelle/tests/unused.res | 30 ++++++++++++++++
 contrib/coccinelle/unused.cocci     | 32 +++++++++++++++++
 contrib/scalar/scalar.c             |  3 +-
 diff.c                              |  2 --
 7 files changed, 119 insertions(+), 10 deletions(-)
 create mode 100644 contrib/coccinelle/tests/unused.c
 create mode 100644 contrib/coccinelle/tests/unused.res
 create mode 100644 contrib/coccinelle/unused.cocci
diff --git a/builtin/fetch.c b/builtin/fetch.c
index ac29c2b1ae3..8a3ae71fed0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1113,7 +1113,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 			      struct fetch_head *fetch_head, struct worktree **worktrees)
 {
 	int url_len, i, rc = 0;
-	struct strbuf note = STRBUF_INIT, err = STRBUF_INIT;
+	struct strbuf note = STRBUF_INIT;
 	const char *what, *kind;
 	struct ref *rm;
 	char *url;
@@ -1281,7 +1281,6 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 
  abort:
 	strbuf_release(&note);
-	strbuf_release(&err);
 	free(url);
 	return rc;
 }
diff --git a/builtin/merge.c b/builtin/merge.c
index d9784d4891c..23170f2d2a6 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -375,7 +375,6 @@ static void reset_hard(const struct object_id *oid, int verbose)
 static void restore_state(const struct object_id *head,
 			  const struct object_id *stash)
 {
-	struct strbuf sb = STRBUF_INIT;
 	const char *args[] = { "stash", "apply", NULL, NULL };
 
 	if (is_null_oid(stash))
@@ -391,7 +390,6 @@ static void restore_state(const struct object_id *head,
 	 */
 	run_command_v_opt(args, RUN_GIT_CMD);
 
-	strbuf_release(&sb);
 	refresh_cache(REFRESH_QUIET);
 }
 
@@ -502,7 +500,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
 {
 	struct commit *remote_head;
 	struct object_id branch_head;
-	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	struct merge_remote_desc *desc;
 	const char *ptr;
@@ -590,7 +587,6 @@ static void merge_name(const char *remote, struct strbuf *msg)
 		oid_to_hex(&remote_head->object.oid), remote);
 cleanup:
 	free(found_ref);
-	strbuf_release(&buf);
 	strbuf_release(&bname);
 }
 
diff --git a/contrib/coccinelle/tests/unused.c b/contrib/coccinelle/tests/unused.c
new file mode 100644
index 00000000000..495ae58ccf1
--- /dev/null
+++ b/contrib/coccinelle/tests/unused.c
@@ -0,0 +1,55 @@
+void test_strbuf(void)
+{
+	struct strbuf sb1 = STRBUF_INIT;
+	struct strbuf sb2 = STRBUF_INIT;
+	struct strbuf sb3 = STRBUF_INIT;
+	struct strbuf sb4 = STRBUF_INIT;
+	struct strbuf sb5;
+	struct strbuf sb6 = { 0 };
+	struct strbuf sb7 = STRBUF_INIT;
+	struct strbuf sb8 = STRBUF_INIT;
+	struct strbuf *sp1;
+	struct strbuf *sp2;
+	struct strbuf *sp3;
+	struct strbuf *sp4 = xmalloc(sizeof(struct strbuf));
+	struct strbuf *sp5 = xmalloc(sizeof(struct strbuf));
+	struct strbuf *sp6 = xmalloc(sizeof(struct strbuf));
+	struct strbuf *sp7;
+
+	strbuf_init(&sb5, 0);
+	strbuf_init(sp1, 0);
+	strbuf_init(sp2, 0);
+	strbuf_init(sp3, 0);
+	strbuf_init(sp4, 0);
+	strbuf_init(sp5, 0);
+	strbuf_init(sp6, 0);
+	strbuf_init(sp7, 0);
+	sp7 = xmalloc(sizeof(struct strbuf));
+
+	use_before(&sb3);
+	use_as_str("%s", sb7.buf);
+	use_as_str("%s", sp1->buf);
+	use_as_str("%s", sp6->buf);
+	pass_pp(&sp3);
+
+	strbuf_release(&sb1);
+	strbuf_reset(&sb2);
+	strbuf_release(&sb3);
+	strbuf_release(&sb4);
+	strbuf_release(&sb5);
+	strbuf_release(&sb6);
+	strbuf_release(&sb7);
+	strbuf_release(sp1);
+	strbuf_release(sp2);
+	strbuf_release(sp3);
+	strbuf_release(sp4);
+	strbuf_release(sp5);
+	strbuf_release(sp6);
+	strbuf_release(sp7);
+
+	use_after(&sb4);
+
+	if (when_strict())
+		return;
+	strbuf_release(&sb8);
+}
diff --git a/contrib/coccinelle/tests/unused.res b/contrib/coccinelle/tests/unused.res
new file mode 100644
index 00000000000..b3b71053ed6
--- /dev/null
+++ b/contrib/coccinelle/tests/unused.res
@@ -0,0 +1,30 @@
+void test_strbuf(void)
+{
+	struct strbuf sb3 = STRBUF_INIT;
+	struct strbuf sb4 = STRBUF_INIT;
+	struct strbuf sb7 = STRBUF_INIT;
+	struct strbuf *sp1;
+	struct strbuf *sp3;
+	struct strbuf *sp6 = xmalloc(sizeof(struct strbuf));
+	strbuf_init(sp1, 0);
+	strbuf_init(sp3, 0);
+	strbuf_init(sp6, 0);
+
+	use_before(&sb3);
+	use_as_str("%s", sb7.buf);
+	use_as_str("%s", sp1->buf);
+	use_as_str("%s", sp6->buf);
+	pass_pp(&sp3);
+
+	strbuf_release(&sb3);
+	strbuf_release(&sb4);
+	strbuf_release(&sb7);
+	strbuf_release(sp1);
+	strbuf_release(sp3);
+	strbuf_release(sp6);
+
+	use_after(&sb4);
+
+	if (when_strict())
+		return;
+}
diff --git a/contrib/coccinelle/unused.cocci b/contrib/coccinelle/unused.cocci
new file mode 100644
index 00000000000..56530498d17
--- /dev/null
+++ b/contrib/coccinelle/unused.cocci
@@ -0,0 +1,32 @@
+// This rule finds sequences of "unused" declerations and uses of a
+// variable, where "unused" is defined to include only calling the
+// equivalent of alloc, init & free functions on the variable.
+@@
+type T;
+identifier I;
+constant INIT_MACRO =~ "^STRBUF_INIT$";
+identifier MALLOC1 =~ "^x?[mc]alloc$";
+identifier INIT_CALL1 =~ "^strbuf_init$";
+identifier REL1 =~ "^strbuf_(release|reset)$";
+@@
+
+(
+- T I;
+|
+- T I = { 0 };
+|
+- T I = INIT_MACRO;
+|
+- T I = MALLOC1(...);
+)
+
+<... when != \( I \| &I \)
+(
+- \( INIT_CALL1 \)( \( I \| &I \), ...);
+|
+- I = MALLOC1(...);
+)
+...>
+
+- \( REL1 \)( \( &I \| I \) );
+  ... when != \( I \| &I \)
diff --git a/contrib/scalar/scalar.c b/contrib/scalar/scalar.c
index 28176914e57..97e71fe19cd 100644
--- a/contrib/scalar/scalar.c
+++ b/contrib/scalar/scalar.c
@@ -687,7 +687,7 @@ static int cmd_diagnose(int argc, const char **argv)
 	int stdout_fd = -1, archiver_fd = -1;
 	time_t now = time(NULL);
 	struct tm tm;
-	struct strbuf path = STRBUF_INIT, buf = STRBUF_INIT;
+	struct strbuf buf = STRBUF_INIT;
 	int res = 0;
 
 	argc = parse_options(argc, argv, NULL, options,
@@ -779,7 +779,6 @@ static int cmd_diagnose(int argc, const char **argv)
 	free(argv_copy);
 	strvec_clear(&archiver_args);
 	strbuf_release(&zip_path);
-	strbuf_release(&path);
 	strbuf_release(&buf);
 
 	return res;
diff --git a/diff.c b/diff.c
index e71cf758861..d4290615aaa 100644
--- a/diff.c
+++ b/diff.c
@@ -1289,7 +1289,6 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
 {
 	static const char *nneof = " No newline at end of file\n";
 	const char *context, *reset, *set, *set_sign, *meta, *fraginfo;
-	struct strbuf sb = STRBUF_INIT;
 
 	enum diff_symbol s = eds->s;
 	const char *line = eds->line;
@@ -1521,7 +1520,6 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
 	default:
 		BUG("unknown diff symbol");
 	}
-	strbuf_release(&sb);
 }
 
 static void emit_diff_symbol(struct diff_options *o, enum diff_symbol s,

-- 
2.37.0.913.g50625c3f077
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 20 of 24 in “cocci: add and apply a rule to find "unused" variables”
  1. cocci: add and apply a rule to find "unused" variablesÆvar Arnfjörð Bjarmason, May 20, 2022
  2. 0/2 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jun 21, 2022
  3. 1/2 cocci: add and apply a rule to find "unused" variablesÆvar Arnfjörð Bjarmason, Jun 21, 2022
  4. Junio C HamanoJun 22, 2022
  5. 2/2 cocci: remove "when strict" from unused.cocciÆvar Arnfjörð Bjarmason, Jun 21, 2022
  6. 0/4 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jul 1, 2022
  7. 1/4 cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason, Jul 1, 2022
  8. Jeff KingJul 1, 2022
  9. Eric SunshineJul 1, 2022
  10. 2/4 cocci: catch unused "strbuf" using an xmalloc() patternÆvar Arnfjörð Bjarmason, Jul 1, 2022
  11. 4/4 cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason, Jul 1, 2022
  12. 3/4 cocci: remove "when strict" from unused.cocciÆvar Arnfjörð Bjarmason, Jul 1, 2022
  13. Eric SunshineJul 1, 2022
  14. Jeff KingJul 1, 2022
  15. 0/6 add and apply a rule to find "unused" init+freeÆvar Arnfjörð Bjarmason, Jul 5, 2022
  16. 2/6 Makefile & .gitignore: ignore & clean "git.res", not "*.res"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  17. 1/6 Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGSÆvar Arnfjörð Bjarmason, Jul 5, 2022
  18. 3/6 cocci: add a "coccicheck-test" target and test *.cocci rulesÆvar Arnfjörð Bjarmason, Jul 5, 2022
  19. 4/6 cocci: have "coccicheck{,-pending}" depend on "coccicheck-test"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  20. 5/6 cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason, Jul 5, 2022
  21. 6/6 cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason, Jul 5, 2022
  22. Junio C HamanoJul 6, 2022
  23. Jeff KingJul 11, 2022
  24. Ævar Arnfjörð BjarmasonJul 11, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.