[FFmpeg-devel] [PATCH] VP8 decoder

[Måns Rullgård]

"Ronald S. Bultje" <rsbultje at gmail.com> writes:

> Hi,
>
> 2010/6/22 M?ns Rullg?rd <mans at mansr.com>:
>> "Ronald S. Bultje" <rsbultje at gmail.com> writes:
>>> +#ifndef PACK4x8
>>> +#define PACK4x8(a,b,c,d) \
>>> + ? ?HAVE_BIGENDIAN ? (a << 24) | (b << 16) | (c << 8) | d : \
>>> + ? ? ? ? ? ? ? ? ? ? (d << 24) | (c << 16) | (b << 8) | a
>>> +#endif
>>
>> This is missing parens everywhere. ?I'm also uncertain whether it
>> might not be better to use #if HAVE_BIGENDIAN instead. ?You never know
>> with compilers...
>
> That one I'm not too worried about, don't we require it to do that
> correctly (symbol elimination) in allcodecs.c?

We do, but I still don't trust them.  This one would go unnoticed if
it didn't do the right thing.

> Brackets around the whole operation added.
>
> +#ifndef PACK4x8
> +#define PACK4x8(a,b,c,d) \
> +    HAVE_BIGENDIAN ? ((a << 24) | (b << 16) | (c << 8) | d) : \
> +                     ((d << 24) | (c << 16) | (b << 8) | a)
> +#endif

Congratulations, you added the only parens you don't need.  You need
parens around each use of the args and around the whole thing.

-- 
M?ns Rullg?rd
mans at mansr.com