[FFmpeg-devel] [PATCH v3] add put_bits_long to avoid undefined behaviour in put_bits

[Michael Niedermayer]

On Sun, Sep 13, 2009 at 03:17:01PM +0200, matthieu castet wrote:
> Hi,
>
> the put_bits can be called with
> 	unsigned int bit_buf = 0;
> 	int bit_left = 32;
> 	int n = 32;
> 	unsigned int value = 0x1ba;
>
> in the case of ALT_BITSTREAM_WRITER and BITSTREAM_WRITER_LE is not defined, 
> we
> will do "bit_buf<<=bit_left;"
> But sizeof(bit_buf) == 32 and bit_left==32, and we are in a undefined 
> behavior[1].
>
> This patch add a put_bits_long for n = 32 and restrict n <= 31 for 
> put_bits.
>
> This new version should fix all the comments made for previous patches.
>
>
> Matthieu
>
> PS : this should fix fate for llvm
>
> [1] C99 standard, ?6.5.7
> If the value of the right operand is negative or is
> greater than or equal to the width of the promoted left operand, the 
> behavior is undefined.
[...]
> @@ -259,6 +259,22 @@
>      put_bits(pb, bits, val & ((1<<bits)-1));
>  }
>  
> +static inline void put_bits_long(PutBitContext *s, int n, unsigned int value)
> +{
> +    if(n <= 31) {
> +        put_bits(s, n, value);
> +    } else {
> +#ifdef ALT_BITSTREAM_READER_LE
> +        put_bits(s, 16, value & 0xffff);
> +        put_bits(s, 16, value >> 16);
> +#else
> +        put_bits(s, 16, value >> 16);
> +        put_bits(s, 16, value & 0xffff);
> +#endif
> +    }
> +
> +}
> +

i dont think this should be static inline or where is it speed critical?

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

> ... defining _GNU_SOURCE...
For the love of all that is holy, and some that is not, don't do that.
-- Luca & Mans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090914/61c60990/attachment.pgp>