[Ffmpeg-devel] [PATCH] ASSERT_BITS

[Michael Niedermayer]

Hi

On Tue, Oct 24, 2006 at 06:00:22PM +0200, Steve Lhomme wrote:
> Hi,
> 
> This patch allow to put asserts inside the bitstream reader to make sure 
> no bits are used outside of what's in memory. (in the past it helped me 
> debug a few buffer overflow).

[...]
> @@ -497,6 +503,9 @@
>          (gb)->cache= name##_cache;\
>          (gb)->buffer_ptr= name##_buffer_ptr;\
>  
> +#   define ASSERT_BITS(gb, num)\
> +        assert((gb)->bit_count + FF_INPUT_BUFFER_PADDING_SIZE*8 >= (num));
> +
>  #ifdef LIBMPEG2_BITSTREAM_READER_HACK
>  
>  #   define UPDATE_CACHE(name, gb)\
> @@ -571,6 +580,9 @@
>          (gb)->cache1= name##_cache1;\
>          (gb)->buffer_ptr= name##_buffer_ptr;\
>  
> +#   define ASSERT_BITS(gb, num)\
> +        assert((gb)->bit_count + FF_INPUT_BUFFER_PADDING_SIZE*8 >= (num));
> +
>  #   define UPDATE_CACHE(name, gb)\
>      if(name##_bit_count > 0){\
>          const uint32_t next= be2me_32( *name##_buffer_ptr );\

these 2 dont look correct, i would rather expect them to compare buffer_ptr
against buffer_end
also why dont you check in and only in UPDATE_CACHE() ? IIRC thats the only
spot where memory gets read (=where a segfault can occur) 


> @@ -631,8 +643,14 @@
>      CLOSE_READER(re, s)
>  }
>  
> +
>  #endif

cosmetic


>  
> +#if !defined(HAVE_ASSERT_BITS)

id suggest #ifndef but thats of course nitpicking and no reason to reject
the patch

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is