Duy Nguyen <[email protected]> writes:

> 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);


Reply via email to