Thread (5 messages) flat view 5 messages, 3 authors, 2018-08-17

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

From: Jeff King <hidden>
Date: 2018-08-17 17:33:11

Possibly related (same subject, not in this thread)

On Fri, Aug 17, 2018 at 10:07:36AM -0700, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help