Re: removing useless if-before-free tests

[Jim Meyering]

Bruno Haible <address@hidden> wrote:
> While it is true that
>
>    if (p != NULL)
>      free (p);
>
> is equivalent to
>
>    free (p);
>
> the latter takes more instructions if there is a significant probability
> that p is NULL. 2 instruction in the caller compared to a call instruction
> and 6 instructions in the callee.
>
> I don't see the point of de-optimizing code that is currently well optimized.
>
> So I'm asking for consideration. Take, for example, lib/gl_carray_list.c.
> Here, in
>
>       if (list->elements != NULL)
>         free (list->elements);
>
> list->elements is NULL quite frequently, since empty lists are quite
> frequent, and every list starts out empty initially. So I want to keep the
> 'if' here.

I hear what you're saying.
I'd like to avoid spending time looking for ways to micro-optimize.
How about if I remove them all, and then restore-with-justification
the ones for which you think there is a significant performance gain?
Then, at least, there will be some indication to future reviewers
why we've kept a seemingly redundant test.

> Another example is tests/test-c-strstr.c:
>
>     if (needle != NULL)
>       free (needle);
>     if (haystack != NULL)
>       free (haystack);

> Here the two variables can only be NULL if there was a memory allocation
> error, which is extremely rare. Therefore I'm in favour of removing the
> 'if' here.

I agree those should go, along with all of the others in tests/*.c.

However, I think *any* redundant if-before-free
should be removed without a second thought, unless
there is actual data to show that keeping it would
result in a significant performance improvement.