[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: removing useless if-before-free tests
From: |
Bruno Haible |
Subject: |
Re: removing useless if-before-free tests |
Date: |
Mon, 18 Feb 2008 19:21:52 +0100 |
User-agent: |
KMail/1.9.1 |
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