[Ffmpeg-devel] [PATCH] from DivX, Part 8: bitstream assert

[Michael Niedermayer]

Hi

On Fri, Dec 16, 2005 at 01:59:20PM -1000, Steve Lhomme wrote:
> This patch adds some assert() in the bitstream reader. That should avoid 
> silly bugs like the ones I'm going to send next (reading more bits than 
> available).
> 
> I know this was discussed before, assert() in C is meant to be used by 
> developpers to stop when a mandatory condition is not met in the code. 
> This allows and ensures errors are handled in the code. And thus that it 
> doesn't crash on the user. (all the crashes in DrDivX OSS are now due to 
> problems in FFMPEG).
> 
> So please, when you develop make sure the assert() is on, that will save 
> a lot of time later.
> 
> 

> Index: libavcodec/bitstream.h
> ===================================================================
> RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/bitstream.h,v
> retrieving revision 1.156
> diff -u -r1.156 bitstream.h
> --- libavcodec/bitstream.h	21 Sep 2005 23:18:52 -0000	1.156
> +++ libavcodec/bitstream.h	16 Dec 2005 23:52:54 -0000
> @@ -390,6 +390,9 @@
>          int name##_index= (gb)->index;\
>          int name##_cache= 0;\
>  
> +#   define ASSERT_BITS(gb, num)\
> +        assert((gb)->index + (num) <= (gb)->size_in_bits);
> +
>  #   define CLOSE_READER(name, gb)\
>          (gb)->index= name##_index;\
>  
> @@ -452,6 +455,9 @@
>          (gb)->cache= name##_cache;\
>          (gb)->buffer_ptr= name##_buffer_ptr;\
>  
> +#   define ASSERT_BITS(gb, num)\
> +        assert((gb)->bit_count >= (num));
> +
>  #ifdef LIBMPEG2_BITSTREAM_READER_HACK
>  
>  #   define UPDATE_CACHE(name, gb)\
> @@ -516,6 +522,9 @@
>          (gb)->cache1= name##_cache1;\
>          (gb)->buffer_ptr= name##_buffer_ptr;\
>  
> +#   define ASSERT_BITS(gb, num)\
> +        assert((gb)->bit_count >= (num));
> +

you forgot to consider FF_INPUT_BUFFER_PADDING_SIZE in these

[...]

-- 
Michael