On Sun, Dec 16, 2012 at 11:45:37AM -0600, Derek Martin wrote:
I noticed something curious while looking at a bit of Mutt code...
The macro FREE() is redundant: it's defined to safe_free(), which
has exactly the same interface, and which itself has two
IIRC, FREE(x) used to be defined as safe_free(&(x)). It was just
a shortcut to avoid having to remember to pass a
pointer-to-a-pointer.
implementations. One is in lib.c, the other in rfc822.c. They are
The rfc822c.c file was originally designed to be self sufficient,
such that it could be reused in other projects. I'm not sure if
this is the case any longer.
different, but similar. The lib.c implementation is this:
void safe_free (void *ptr) /* __SAFE_FREE_CHECKED__ */
{
void **p = (void **)ptr;
if (*p)
{
free (*p); /* __MEM_CHECKED__ */
*p = 0;
}
}
Is there any specific reason the implementation was written thusly?
Granted, this is a pretty minor issue... But still, this all seems
kind of gross. First, there really ought to be one implementation.
Second, the above implementation has some redundancies:
- It requires a function call
It has been awhile since I've done any performance testing of
Mutt, but the overhead involved here was not significant.
- Calling free(NULL) is a noop--perfectly safe (at least since ANSI
C, I don't have a copy of K&R handy)
I don't remember the reason for this. Back in 1995 when I started
writing it I'm not sure that all target platforms had ANSI C
support.
- the extra cast seems silly; ptr is required to be **ptr, it should
be declared that way.
I'm not much of a fan of function-like macros in general, but this
case seems like exactly when you'd want to use one. I think this
should be replaced with:
#define FREE(x)\
free((x));\
x = NULL
It should be free(*(x)), since a ** is passed. And if you want to
avoid a function, you need to make sure it can be called in and
if- or else- block without the braces, like this:
#define FREE(x)\
do { free(*(x)); *x = NULL } while(0)
At this point you may well just consider using an inlined
function.
me
