[FFmpeg-devel] [Patch] Fix for static leaks in h264.c

[Art Clarke]

On Sun, Jun 29, 2008 at 6:52 AM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> >
> > > Of course for macros which do look like functions i agree that the
> do/while
> > > should be added, but anything written in all upper case does not look
> like
> > > a function.
> >
> > Function or not, the way it is used, it looks like a single
> > *statement*, and should behave like one.  It's a very simple way to
> > avoid hard to find bugs, especially since you encourage if statements
> > without {}.
>
> macros are NOT simple single statements.
> Debugging a MAX(a++,b++) tends to nicely remind one of that, especially
> when it was written lowercase and looking like a function.
>
> IMHO if you want a function, write it as a function and not a macro.


Here's my opinion on the do {...} while(0) in this case.

In general, I'm a stickler for putting do {...} while(0) on macros that are
only macros to avoid taking a function call hit.  In those cases (i.e. the
method could be rewritten as a function, but is only a macro for performance
reasons), the do-while provides a valuable service: it let's you promote
(demote?) from a macro to a function later if need be and be relatively sure
most calling code will work without compilation errors.

In this specific case, these macros could not be rewritten as functions --
they are explicitly allocating data-segment space at their invocation spot,
in the calling function.  Therefore they will NEVER become functions.  In
that case, the do{...}while merely removes some potential compilation
oddness (i.e. it means I can't ever invoke the macro without a semi-colon),
but doesn't provide much future value for converting to a function.  In this
case, my usual rule is to follow the conventions in the code I'm editing.

And based on that, I violated my own rule, and shouldn't have added the
do{...}while(0) because (a) Michael wrote the original code and (b) it
doesn't add enough value.

So, I will submit a patch without the do-while.  I'm not doing it right now
because my Linux dev environment is being shipped across the US (I'm in the
process of a cross-country move), but once that's back up in late July, I'll
submit another one.

$0.02.

- Art