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

Art Clarke aclarke
Fri Jul 18 22:56:44 CEST 2008


On Wed, Jul 2, 2008 at 12:45 PM, Art Clarke <aclarke at vlideshow.com> wrote:

> 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
>

As promised (now that I have my Linux dev environment up and running after a
cross country move), here is the fix for the static leak in h264.c without
the "do { ... } while(0)" around the macros.  Michael, your call, but I
think this has addressed all objections.

- Art
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080718/976e17c2/attachment.txt>



More information about the ffmpeg-devel mailing list