[Ffmpeg-devel] [PATCH] ASSERT_BITS

[Steve Lhomme]

Michael Niedermayer wrote:
> 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) 

As I don't have any way to test the other 2, I just add it for 
ALT_BITSTREAM_READER.

I call it before 'some' calls to UPDATE_CACHE, not all. But I don't 
think it's good to call it directly inside UPDATE_CACHE, otherwise it 
might degrade performance a lot. Now if someone wants to improve it, 
feel free :)

Steve
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: assert_bits.v2.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061101/df211dfc/attachment.asc>