Re: [PATCH v4 4/6] coccinelle: add a rule to make "expression" code use FREE_AND_NULL()

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

Late reply, sorry. What version of coccinelle are you running? I have
1.0.4 (from Debian) and can't get this to produce the same results as
what I have.

On top of next, I did:

        Revert "*.[ch] refactoring: make use of the FREE_AND_NULL() macro"
        Revert "coccinelle: make use of the "expression" FREE_AND_NULL() rule"
        Revert "coccinelle: make use of the "type" FREE_AND_NULL() rule"

And then generated the patch as usual with `make coccicheck`, and
applied it. This has your change.

I then re-applied the manual "*.[ch] refactoring" changes

This results in this diff with next:
...

These are all cases where we now miss things that should use
FREE_AND_NULL(), e.g.:

    diff --git a/builtin/am.c b/builtin/am.c
    index c973bd96dc..2f89338ed7 100644
    --- a/builtin/am.c
    +++ b/builtin/am.c
    @@ -484,7 +484,8 @@ static int run_applypatch_msg_hook(struct am_state *state)
            ret = run_hook_le(NULL, "applypatch-msg", am_path(state, "final-commit"), NULL);

            if (!ret) {
    -               FREE_AND_NULL(state->msg);
    +               free(state->msg);
    +               state->msg = NULL;
                    if (read_commit_msg(state) < 0)
                            die(_("'%s' was deleted by the applypatch-msg hook"),
                                    am_path(state, "final-commit"));

So it looks to me like removing the "type T" rule breaks a lot of
things, but that the change you made to the expression rule is good, and
we should do that for the "type" rule as well. Your commit says the
"expression rule already covers it", but this doesn't seem to be the
case at all.

OK, if an older version of the tool that is otherwise still usable
needs two rules, let's keep the "type T" one by reverting the change
out of 'next' and then have a patch that only rearranges the lines,
something like the attached.

-- >8 --
From: René Scharfe <redacted>
Date: Sun, 25 Jun 2017 10:01:04 +0200
Subject: [PATCH] coccinelle: polish FREE_AND_NULL rules

There are two rules for using FREE_AND_NULL in free.cocci, one for
pointer types and one for expressions.  Both cause coccinelle to remove
empty lines and even newline characters between replacements for some
reason; consecutive "free(x);/x=NULL;" sequences end up as multiple
FREE_AND_NULL calls on the same time.

Rearrange the lines to place the addition of FREE_AND_NULL between
the removals, which causes coccinelle to leave surrounding
whitespace untouched.

Signed-off-by: Rene Scharfe <redacted>
Signed-off-by: Junio C Hamano <redacted>
---
 contrib/coccinelle/free.cocci | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/coccinelle/free.cocci b/contrib/coccinelle/free.cocci
index f2d97e755b..179c185d85 100644
--- a/contrib/coccinelle/free.cocci
+++ b/contrib/coccinelle/free.cocci

@@ -15,12 +15,12 @@ type T;
 T *ptr;
 @@
 - free(ptr);
-- ptr = NULL;
 + FREE_AND_NULL(ptr);
+- ptr = NULL;

 @@
 expression E;
 @@
 - free(E);
-- E = NULL;
 + FREE_AND_NULL(E);
+- E = NULL;