All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH] cocci: add and apply a rule to find "unused" variables
Date: Fri, 20 May 2022 13:57:05 +0200	[thread overview]
Message-ID: <patch-1.1-7d90f26b73f-20220520T115426Z-avarab@gmail.com> (raw)

Add a coccinelle rule to remove variable initialization followed by
calling a "release" function. This rule automatically finds the sort
of issue patched in[1], and more.

We happened to only have occurrences of strbuf_release() matching this
rule, but manual testing reveals that it'll find e.g. the same pattern
if "string_list_clear()" were used instead.

1. https://lore.kernel.org/git/042d624b8159364229e95d35e9309f12b67f8173.1652977582.git.gitgitgadget@gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---

This overlaps but cleanly merges with the small series that [1] is in,
but just adding a coccinelle rule to catch this seemed like a good
thing to have. We even have another such case in builtin/merge.c, near
the change made in [1]!

FWIW I wrote this working rule too, but there were no in-tree hits for
it, so I didn't include it:

	// Unused declaration + malloc + free()
	@@
	identifier I;
	type T;
	// malloc(), xmalloc(), calloc() etc.
	identifier MALLOC =~ "^x?[mc]alloc$";
	@@

	(
	- T *I;
	  ... when != I
	- I = MALLOC(...);
	|
	- T *I = MALLOC(...);
	)
	  ... when != I
	- free(I);

 builtin/fetch.c                 |  3 +--
 builtin/merge.c                 |  4 ----
 builtin/repack.c                |  2 --
 contrib/coccinelle/unused.cocci | 15 +++++++++++++++
 diff.c                          |  2 --
 5 files changed, 16 insertions(+), 10 deletions(-)
 create mode 100644 contrib/coccinelle/unused.cocci

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e3791f09ed5..600c28fdb75 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 f178f5a3ee1..bb6b0580659 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);
 }
 
@@ -501,7 +499,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;
@@ -589,7 +586,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/builtin/repack.c b/builtin/repack.c
index d1a563d5b65..52f8450f1be 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -609,7 +609,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;
@@ -955,7 +954,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/unused.cocci b/contrib/coccinelle/unused.cocci
new file mode 100644
index 00000000000..52c23e15310
--- /dev/null
+++ b/contrib/coccinelle/unused.cocci
@@ -0,0 +1,15 @@
+// Unused init assignment + release()
+@@
+identifier I;
+type T;
+constant INIT =~ "_INIT";
+// stbuf_release(), string_list_clear() etc.
+identifier REL1 =~ "^[a-z_]*_(release|clear|free)$";
+// release_patch(), clear_pathspec() etc.
+identifier REL2 =~ "^(release|clear|free)_[a-z_]*$";
+@@
+
+- T I = INIT;
+  <+... when != \( I \| &I \)
+- \( REL1 \| REL2 \)(&I, ...);
+  ...+>
diff --git a/diff.c b/diff.c
index ef7159968b6..57997937071 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.36.1.960.g7a4e2fc85c9


             reply	other threads:[~2022-05-20 11:57 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-20 11:57 Ævar Arnfjörð Bjarmason [this message]
2022-05-20 12:16 ` [cocci] Matching VAR followed by foo(VAR) with no other use of VAR in the function Ævar Arnfjörð Bjarmason
2022-05-20 13:29   ` Julia Lawall
2022-06-21 22:44     ` Ævar Arnfjörð Bjarmason
2022-06-21 22:44 ` [PATCH v2 0/2] add and apply a rule to find "unused" init+free Ævar Arnfjörð Bjarmason
2022-06-21 22:44   ` [PATCH v2 1/2] cocci: add and apply a rule to find "unused" variables Ævar Arnfjörð Bjarmason
2022-06-22 16:02     ` Junio C Hamano
2022-06-21 22:44   ` [PATCH v2 2/2] cocci: remove "when strict" from unused.cocci Ævar Arnfjörð Bjarmason
2022-07-01 10:30   ` [PATCH v3 0/4] add and apply a rule to find "unused" init+free Ævar Arnfjörð Bjarmason
2022-07-01 10:30     ` [PATCH v3 1/4] cocci: add and apply a rule to find "unused" strbufs Ævar Arnfjörð Bjarmason
2022-07-01 18:04       ` Jeff King
2022-07-01 19:55       ` Eric Sunshine
2022-07-01 10:30     ` [PATCH v3 2/4] cocci: catch unused "strbuf" using an xmalloc() pattern Ævar Arnfjörð Bjarmason
2022-07-01 10:30     ` [PATCH v3 3/4] cocci: remove "when strict" from unused.cocci Ævar Arnfjörð Bjarmason
2022-07-01 21:33       ` Eric Sunshine
2022-07-01 10:30     ` [PATCH v3 4/4] cocci: generalize "unused" rule to cover more than "strbuf" Ævar Arnfjörð Bjarmason
2022-07-01 18:09     ` [PATCH v3 0/4] add and apply a rule to find "unused" init+free Jeff King
2022-07-05 13:46     ` [PATCH v4 0/6] " Ævar Arnfjörð Bjarmason
2022-07-05 13:46       ` [PATCH v4 1/6] Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS Ævar Arnfjörð Bjarmason
2022-07-05 13:46       ` [PATCH v4 2/6] Makefile & .gitignore: ignore & clean "git.res", not "*.res" Ævar Arnfjörð Bjarmason
2022-07-05 13:46       ` [PATCH v4 3/6] cocci: add a "coccicheck-test" target and test *.cocci rules Ævar Arnfjörð Bjarmason
2022-07-05 13:46       ` [PATCH v4 4/6] cocci: have "coccicheck{,-pending}" depend on "coccicheck-test" Ævar Arnfjörð Bjarmason
2022-07-05 13:46       ` [PATCH v4 5/6] cocci: add and apply a rule to find "unused" strbufs Ævar Arnfjörð Bjarmason
2022-07-05 13:47       ` [PATCH v4 6/6] cocci: generalize "unused" rule to cover more than "strbuf" Ævar Arnfjörð Bjarmason
2022-07-06 19:30       ` [PATCH v4 0/6] add and apply a rule to find "unused" init+free Junio C Hamano
2022-07-11  9:41       ` Jeff King
2022-07-11 10:54         ` Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-1.1-7d90f26b73f-20220520T115426Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.