On Fri, Aug 17, 2018 at 10:07:36AM -0700, Junio C Hamano wrote: > Junio C Hamano <gitster@xxxxxxxxx> 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
