"Ronald S. Bultje" <[email protected]> writes:

f> Hi,
>
> 2011/12/15 Måns Rullgård <[email protected]>
>
>> >> @@ -230,7 +263,12 @@ static inline int get_bits_count(const
>> GetBitContext
>> >> *s) {
>> >> >  static inline void skip_bits_long(GetBitContext *s, int n){
>> >> >      OPEN_READER(re, s);
>> >> >      re_bit_count += n;
>> >> > +#if UNCHECKED_BITSTREAM_READER
>> >> >      re_buffer_ptr += re_bit_count>>5;
>> >> > +#else
>> >> > +    re_buffer_ptr = (re_bit_count >> 5) < (const uint32_t *)
>> s->buffer_end - re_buffer_ptr ?
>> >> > +                    re_buffer_ptr + (re_bit_count >> 5) : (const
>> uint32_t *) s->buffer_end;
>> >> > +#endif
>> >>
>> >> This is again guaranteed to fall within the padding region, so whatever
>> >> variant gives better code is OK to use here.
>> >
>> > gcc messes up and does 4 more instructions. Fortunately it's not terribly
>> > important, but I'll stick to this:
>> >
>> > [..]
>> >       re_buffer_ptr += re_bit_count>>5;
>> > + #if !UNCHECKED_BITSTREAM_READER
>> > +     re_buffer_ptr = FFMIN(re_buffer_ptr, s->buffer_end);
>> > + #endif
>>
>> Gah, I'm stupid.  This is skip_bits_long(), which can be called with any
>> value.  It must protect against overflow.
>
> No you're actually right, but differently; re_bit_count is never >31.

The &= 31 is after the addition, so it can have any value.  If that were
not the case, skip_bits_long() would not work at all.

-- 
Måns Rullgård
[email protected]
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to