[FFmpeg-devel] [PATCH] ff_split_xiph_headers returns broken header_len < 0

[Reimar Döffinger]

On Tue, Apr 15, 2008 at 08:47:03PM +0200, Michael Niedermayer wrote:
> > > > With better aligning.
> > > 
> > > > Index: libavcodec/xiph.c
> > > > ===================================================================
> > > > --- libavcodec/xiph.c	(revision 12807)
> > > > +++ libavcodec/xiph.c	(working copy)
> > > > @@ -34,17 +34,24 @@
> > > >              extradata += header_len[i];
> > > >          }
> > > >      } else if (extradata[0] == 2) {
> > > 
> > > > +        int overall_len = 0;
> > > >          for (i=0,j=1; i<2; i++,j++) {
> > > >              header_len[i] = 0;
> > > >              for (; j<extradata_size && extradata[j]==0xff; j++) {
> > > > +                if (overall_len > extradata_size - (0xff + 1))
> > > > +                    return -1;
> > > > +                overall_len   += 0xff + 1;
> > > >                  header_len[i] += 0xff;
> > > >              }
> > > >              if (j >= extradata_size)
> > > >                  return -1;
> > > >  
> > > > +            if (overall_len > extradata_size - (extradata[j] + 1))
> > > > +                return -1;
> > > > +            overall_len   += extradata[j] + 1;
> > > >              header_len[i] += extradata[j];
> > > 
> > > int overall_len = 1;
> > > for (i=0,j=1; i<2; i++,j++) {
> > >     header_len[i] = 0;
> > >     for (; overall_len <= extradata_size && extradata[j]==0xff; j++) {
> > >         overall_len   += 0xff + 1;
> > >         header_len[i] += 0xff;
> > >     }
> > >     overall_len   += extradata[j];
> > 
> > I assume you forgot the +1 here?
> 
> no, look up "int overall_len = 1"

Maybe I am missing something, but I think that just takes care of the
j=1 (which I forgot) but not of the j++ of the outer loop.
Initializing overall_len to 3 instead would be possible as well, though
I think it would be somewhat at the cost of readability...