Re: [PATCH] refactor various if (x) FREE_AND_NULL(x) to just FREE_AND_NULL(x)

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 17, 2018, 16:53 UTC
Message-ID: <xmqqpnyhaq93.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180817151012.GA20262@duynguyen.home>

Duy Nguyen <pclouds@gmail.com> writes:

Show 18 quoted lines

> Just fyi this seems to do the trick. Although I'm nowhere good at
> coccinelle to say if we should include this (or something like it)
>
> -- 8< --
> diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
> index 4490069df9..f8e018d104 100644
> --- a/contrib/coccinelle/free.cocci
> +++ b/contrib/coccinelle/free.cocci
> @@ -16,3 +16,9 @@ expression E;
>  - free(E);
>  + FREE_AND_NULL(E);
>  - E = NULL;
> +
> +@@
> +expression E;
> +@@
> +- if (E) { FREE_AND_NULL(E); }
> ++ FREE_AND_NULL(E);

It is a bit sad that

	- if (E)
	  FREE_AND_NULL(E);

is not sufficient to catch it. Shouldn't we be doing the same for regular free(E) as well? IOW, like the attached patch.

 contrib/coccinelle/free.cocci | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
index 4490069df9..f748bcfe30 100644
--- a/contrib/coccinelle/free.cocci
+++ b/contrib/coccinelle/free.cocci
@@ -16,3 +16,27 @@ expression E;
 - free(E);
 + FREE_AND_NULL(E);
 - E = NULL;
+
+@@
+expression E;
+@@
+- if (E)
+  FREE_AND_NULL(E);
+
+@@
+expression E;
+@@
+- if (E) { free(E); }
++ free(E);
+
+@@
+expression E;
+@@
+- if (!E) { free(E); }
++ free(E);
+
+@@
+expression E;
+@@
+- if (E) { FREE_AND_NULL(E); }
++ FREE_AND_NULL(E);