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

[Jeff King <peff@peff.net>]

On Fri, Aug 17, 2018 at 10:07:36AM -0700, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > 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.
> > ...
> 
> And revised even more to also spell "E" as "E != NULL" (and "!E" as
> "E == NULL"), which seems to make a difference, which is even more
> sad.  I do not want to wonder if I have to also add "NULL == E" and
> other variants, so I'll stop here.

I think it makes sense that these are all distinct if you're using
coccinelle to do stylistic transformations between them (e.g., enforcing
curly braces even around one-liners).

I wonder if there is a way to "relax" a pattern where these semantically
equivalent cases can all be covered automatically. I don't know enough
about the tool to say.

I guess one way to do it would be to normalize the style in one rule
(e.g., always "!E" instead of "E == NULL"), and then you only have to
write the FREE_AND_NULL rule for the normalized form. For a single case
like this, the end result is about the same number of rules, but in the
long term it saves us work when we have a similar transformation.

-Peff