Re: removing useless if-before-free tests

[Bruno Haible]

Jim Meyering wrote:
> $ mkid && ./build-aux/useless-if-before-free -l $(lid -knone free)|tr '\0' 
> '\n'
> lib/csharpcomp.c
> lib/fchdir.c
> lib/free.c
> lib/fstrcmp.c
> lib/gc-gnulib.c
> lib/getaddrinfo.c
> lib/gl_carray_list.c
> lib/localcharset.c
> lib/lock.c
> lib/printf-parse.c
> lib/putenv.c
> lib/relocatable.c
> lib/save-cwd.c
> lib/striconv.c
> lib/vasnprintf.c
> tests/test-c-strcasestr.c
> tests/test-c-strstr.c
> tests/test-mbscasestr1.c
> tests/test-mbscasestr2.c
> tests/test-mbsstr1.c
> tests/test-mbsstr2.c
> tests/test-memmem.c
> tests/test-strcasestr.c
> tests/test-striconv.c
> tests/test-striconveh.c
> tests/test-striconveha.c
> tests/test-strstr.c

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.

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.

You might think that these cases are neglectable. Well, not always.
In CLN I have inline wrappers of functions like memcpy. I once measured
the speed difference between CLN built with

  static inline void my_memcpy (void *dest, void *src, size_t n)
  {
    if (n > 0)
      memcpy (dest, src, n);
  }

and CLN built with

  static inline void my_memcpy (void *dest, void *src, size_t n)
  {
    memcpy (dest, src, n);
  }

The difference was significant (5% or something like that) - because
empty sequences of digits occur quite often.

Bruno