Bug 740413

another alternative would be:

char *do_free(char * buf)
{
    if (buf != NULL) {
        PR_Free(buf);
        buf = NULL;
    }
    return buf;
}

...

buf = do_free(buf);

and then macro can be defined like this:

#define DO_FREE(foo) { foo = do_free(foo); }

Please check with SSSD team on the best approach.

Assuming that PR_Free(buf) can never fail (aka will never return without freeing memory) then the best approach is probably to just use a macro like

#define do_zfree(ptr) do { \
        do_free(ptr); \
        ptr = NULL; \
    } while(0)

(This construct allows you to make the macro look like a standard function, since it must be followed by a semicolon).

So in your code you would just call
do_zfree(ptr);


Finally, your if (buf != NULL) check is probably in the wrong place. PR_Free() should be designed so that it's a no-op if it is passed NULL. If it's not, that's probably a bug.

> Finally, your if (buf != NULL) check is probably in the wrong place. PR_Free()
> should be designed so that it's a no-op if it is passed NULL. If it's not,
> that's probably a bug.

I think PR_xxx is an external interface so we can't do any assumptions about it thus need to always check before calling it. IMO with this twist the check is required and in the right place.

On the other part... Ah, I forgot about the while(0) and the issue with ";" not being a noop on some compilers. Is it still the case and should still be considered? What is the chance to ever encounter such constraint especially in the Dogtag project?

(In reply to comment #3)
> On the other part... Ah, I forgot about the while(0) and the issue with ";" not
> being a noop on some compilers. Is it still the case and should still be
> considered? What is the chance to ever encounter such constraint especially in
> the Dogtag project?

The purpose of wrapping a multi-line macro in a "do { } while(0)" (no semicolon) is to make the macro usable as a regular statement, not a compound statement.

When the while(0) is terminated with a semicolon, the "do { } while(0);" block becomes a compound statement again.

We've had a couple of bad macro definitions in SSSD as well, a patch is on the list.

I thought that the only reason is that if you by mistake put extra ";" after the macro that ends with ";" some of the old compilers would choke. My point that with the modern compilers that should not be the case and second ";" will be treated as no op. 
Is there any reason other than make it look like a function and allow to put the ";" at the end with any compiler why "do { } while(0)" is used?

We might actually be talking about the same thing, but I think it applies to modern compilers as well.

Consider the following program:

-----
#include <stdio.h>

#define PRINT(i) do {          \
    printf("i is %d\n", i);    \
} while(0)                     \

int main(void)
{
    int i = 5;

    if (i > 0)
        PRINT(i);
    else
        PRINT(i*-1);

    return 0;
}
-------

This compiles fine, but if you used "while(0);", the code no longer compiles, because the macro would expand to:

-------
if (i > 0)
  do { printf("i is %d\n", i); } while(0); ;
-------

which is a compound ("multiline") statement that must be enclosed in { }