Re: [PATCH] maint: enable the sc_avoid_if_before_free syntax-check

[Jim Meyering]

Bruno Haible wrote:

> Hi Jim,
>
>> > Nowadays little code remains that is written for K&R C, and few minds 
>> > program
>> > for K&R C. Therefore the "if before free" cases that are remaining are
>> > more likely to be intentional than before.
>>
>> If only that were true.  I've just removed several hundreds of
>> such uses from glusterfs.  From what I've seen, the majority of
>> C/C++ developers attempt to avoid free(p) when p is NULL.
>
> For C hackers, the p != NULL test may be intentional: they may want to save
> a function call.

In my experience, most prefer to avoid the unnecessary code,
because free-calling code is rarely performance-critical enough
to worry about the overhead of a single function call.  Even if
they "worry" about it, I'll bet that profiling would show it
is never anywhere near the top.

On the other hand, there is clear benefit (reduced line count and
complexity) from removing the unneeded conditional.

> Whereas C++ programmers often add this test for a different reason:
> C++ APIs are often not well documented (because C++ tends to make you
> create huge, thoughtless APIs, not small and intelligent APIs), and so
> the caller of a function never really knows whether NULL is a possible
> return value. When you have gotten a core dump because your invocation
> of a small function returned NULL instead of an empty list, 5 or 10 times
> in the same week, you learn to add   if (p != NULL)   protections everywhere,
> even where it's not needed.

The idea of using the useless-if-before-free script it to identify
free-like functions and then to enforce the policy of removing all
unnecessary tests before those calls.  I.e., you look up the semantics
of a function once, and if it's free-like, add it to a list.  Then,
there is no need to wonder.

Of course, if it's C++ and you are stuck with functions all named
"free" but with varying NULL-handling semantics, then you cannot
distinguish with the useless-if-before-free's limited regexp-based
matching.  But then you have bigger problems...