[FFmpeg-cvslog] r19991 - trunk/libavcodec/vorbis_dec.c

[Michael Niedermayer]

On Wed, Sep 23, 2009 at 03:08:48PM +0200, Reimar D?ffinger wrote:
> On Wed, Sep 23, 2009 at 02:50:41PM +0200, Michael Niedermayer wrote:
> > On Wed, Sep 23, 2009 at 02:22:21PM +0200, Reimar D?ffinger wrote:
> > > On Wed, Sep 23, 2009 at 02:17:54PM +0200, michael wrote:
> > > > Author: michael
> > > > Date: Wed Sep 23 14:17:54 2009
> > > > New Revision: 19991
> > > > 
> > > > Log:
> > > > Check masterbook index and subclass book index.
> > > > 14_floor_masterbook_index.patch by chrome
> > > > 
> > > > Modified:
> > > >    trunk/libavcodec/vorbis_dec.c
> > > > 
> > > > Modified: trunk/libavcodec/vorbis_dec.c
> > > > ==============================================================================
> > > > --- trunk/libavcodec/vorbis_dec.c	Wed Sep 23 14:09:33 2009	(r19990)
> > > > +++ trunk/libavcodec/vorbis_dec.c	Wed Sep 23 14:17:54 2009	(r19991)
> > > > @@ -487,13 +487,23 @@ static int vorbis_parse_setup_hdr_floors
> > > >                  AV_DEBUG(" %d floor %d class dim: %d subclasses %d \n", i, j, floor_setup->data.t1.class_dimensions[j], floor_setup->data.t1.class_subclasses[j]);
> > > >  
> > > >                  if (floor_setup->data.t1.class_subclasses[j]) {
> > > > -                    floor_setup->data.t1.class_masterbook[j]=get_bits(gb, 8);
> > > > +                    int bits=get_bits(gb, 8);
> > > > +                    if (bits>=vc->codebook_count) {
> > > > +                        av_log(vc->avccontext, AV_LOG_ERROR, "Masterbook index %d is out of range.\n", bits);
> > > > +                        return 1;
> > > > +                    }
> > > > +                    floor_setup->data.t1.class_masterbook[j]=bits;
> > > >  
> > > >                      AV_DEBUG("   masterbook: %d \n", floor_setup->data.t1.class_masterbook[j]);
> > > >                  }
> > > >  
> > > >                  for(k=0;k<(1<<floor_setup->data.t1.class_subclasses[j]);++k) {
> > > > -                    floor_setup->data.t1.subclass_books[j][k]=(int16_t)get_bits(gb, 8)-1;
> > > > +                    int16_t bits=get_bits(gb, 8)-1;
> > > > +                    if (bits!=-1 && bits>=vc->codebook_count) {
> > > > +                        av_log(vc->avccontext, AV_LOG_ERROR, "Subclass book index %d is out of range.\n", bits);
> > > > +                        return 1;
> > > > +                    }
> > > > +                    floor_setup->data.t1.subclass_books[j][k]=bits;
> > > >  
> > > >                      AV_DEBUG("    book %d. : %d \n", k, floor_setup->data.t1.subclass_books[j][k]);
> > > >                  }
> > > 
> > > I feel compelled that these checks, are inconsistent with the other
> > > checks due to using that bits intermediate variable (in particular these
> > > also couldn't use the macro I proposed because of that)...
> > 
> > do you think the intermediate is unneeded ?
> 
> I think so. The whole thing relies on data.t1.subclass_books... etc not
> being used at all after the return, since if codebook_count is 0 the
> default value of 0 it already has is outside the range, too!
> Or am I missing some effect that intermediate variable might have?
> Also as far as I checked, returning for these functions indeed ensures
> nothing more is done at all.
> 
> > > I also think there is some inconsistency as in whether return 1 or
> > > return -1 is use...
> > 
> > yes, lets see what i can do about that ...
> 
> I don't necessarily meant to tell _you_ to do something about it.

it was just an search & replace with 31 clicks on ok, 2 where not ok


> I guess I can find some time to help out, too, though I would have to
> have an idea what is desired (and then you might actually be faster just
> cleaning it up yourself than explaining to me how you want it ;-) ).

it can still be improved, for example ENOMEM and such could be used where
appropriate

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

In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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-cvslog/attachments/20090923/ef45bc6e/attachment.pgp>