Re: cbdataFree() in -HEAD ?

The original , approach is an expression, and is the whole point of it.

The difference between the "do { statement } while(0)" as proposed by
Adrian and "if (x) { ... }" as currently used for safe_free is what
happens when the macro is used inside other flow structures without
parens.

A small example to illustrate the danger of the safe_free definition:

if (something)
   safe_free(variable)
else
   something_else();

Explanation is put at the far end of this message if you don't see the
subtle error here...

I have been brought up with the idea that to avoid such issues one
should make use of expressions when making logics in macros, but
apparently not all compilers are happy with that.. what a mess.

Thinking a bit further, I think the following is an approach that should
fit all:

#define cbdataFree(x) \
        ( (x) = ((x) != NULL ? cbdataInternalFree(x) : NULL) )

and make cbdataInternalFree return NULL and not check the given pointer
for NULL..

No, C is not really meant to do things like this, but anyway ;-)

/Henrik

Alex Rousskov wrote:

> Yeah. The above statement (and the one I suggested) are probably
> illegal inside parens of "for (a;b;c)" or "while()" because it is not
> an operator like "(a,b)" you used to have, but a statement. I am not a
> C guru though.
>
> Alex.
>
> P.S. On the bright side, we will not have these problems in Squid 3.0
> where we could have references instead of these ugly macros. :)
> template <class T> inline
> void safeDelete(T *&x) {
> delete x;
> x = 0;
> }

Note the missing ;. This causes the meaning of the whole block to change
quite radically. something_else() will get executed if something is true
and variable is NULL. The block expands to

  if (something)
     if (variable) {
         free(variable);
         variable = NULL;
     }
     else
         something_else();

Which is quite different from the intended..

Luckily GCC will warn about the if ambiguity here, but not compilers
do...

/Henrik
Received on Tue Mar 13 2001 - 15:47:21 MST