On Wed, Jun 28 2017, Junio C. Hamano jotted:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> 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 <l.s.r@xxxxxx>
> 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 <l.s.r@xxxxxx>
> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
> ---
> 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;
Looks good.
