[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Jim Meyering
Subject: Re: [PATCH] maint: enable the sc_avoid_if_before_free syntax-check
Date: Fri, 13 Jul 2012 08:13:08 +0200

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...

reply via email to

[Prev in Thread] Current Thread [Next in Thread]