[ntp:hackers] DPRINTF rumination condensed

Dave Hart davehart at davehart.com
Sun Feb 8 11:09:16 UTC 2009


Dr. Mills, Frank, hackers, consider this hypothetical
snippet of ntp code:

if (a) 
     DPRINTF(3, ("a!\n"));
else {
     DPRINTF(3, ("we're screwed\n"));
     exit(-1);
}

Leave aside any suggestion there should be braces used for
one-line if/else clauses, for as far as I can tell ntp
source style omits them liberally.

There are two problems with the code above.  DPRINTF
evaluates to nothing in the non-debug case, so this code
won't compile without DEBUG defined.  It might take some ntp
developers a long time between when they make a change and
when they next try a nodebug build (let alone run the
resulting binary).  

More insidious is how the code behaves when DEBUG is
defined.  The else is actually associated with first
DPRINTF's dangling if statement (see include/ntp_debug.h),
so this code will print "we're screwed" only if a is nonzero
and debug < 3.  If a is zero, neither DPRINTF executes.

Based on half-baked recollections from prior work improved
by discussion with Harlan, I'm proposing the following
improved version that should behave less like macro
shorthand and more like a function.  Input solicited.

#ifdef DEBUG
#define DPRINTF(_lvl_, _arg_)              \
     do {                                 \
           if (debug >= (_lvl_))           \
                printf _arg_;              \
     } while (0)
#else
#define DPRINTF(_lvl_, _arg_)   do {} while (0)
#endif

Why "do {} while (0)" you might ask.  {} alone might be
equivalent since ntp requires ANSI C, and in fact my
compiler was happy using that more obvious choice for the
non-debug case.  The assumption in using the longer form is
that some compilers might not like "{};" as a freestanding
statement when you combine the DPRINTF nodebug macro with
the semicolon following DPRINTF() in the source, but would
have no problem with "do {} while  (0)".

This is assuming that every compiler is going to be smart
enough to optimize away the dummy statement.  That's a safe
bet, right?  I'm having a hard time imagining even crusty
compilers would generate any code for do {} while (0).

Cheers,
Dave Hart

[forcing text-only as the list and gmail both seemed to
prefer to do their own HTML -> text conversion resulting in
lots of extra white space... either that or my mailer is
sending a whacky plain text part]


More information about the hackers mailing list