Re: [xine-devel] Re: [xine-cvs] CVS: xine-lib/src/xine-engine demux.c,1.28,1.29

[Marco <M.Z...@fr...>]

On Tuesday 06 May 2003 15:22, Bastien Nocera wrote:
> On Tue, 2003-05-06 at 14:13, Marco Zühlke wrote:
> > On Tuesday 06 May 2003 14:06, Bastien Nocera wrote:
> > > On Tue, 2003-05-06 at 11:34, Marco Zühlke wrote:
> > > > On Tuesday 06 May 2003 01:02, Bastien Nocera wrote:
> > >
> > > <snip>
> > >
> > > > Hello Bastian,
> > > >
> > > > Are you sure that this fix is correct ??
> > > >
> > > > The 3rd paramter to get_optional_data is the data_type
> > > > to indicate which kind of optional data is requested.
> > > > The function alway fills the buffer (buf) with MAX_PREVIEW_SIZE
> > > > bytes.
> > > >
> > > > In my opinion your fix is wrong.
> > > > Do you have a special bug that you fix with this change?
> > >
> > > Really fixed now:
> > > + read_size = MIN (read_size, size);
> > >
> > > Thanks for taking me on my bug.
> >
> > That's looks good to me.
> >
> > Maybe the memcpy should then only copy read_size bytes ?
>
> Yes, you're right, can you fix that as well?

I have no cvs account so I can't !!

>
> > -------------------------------------------------------
> >   } else if (input->get_capabilities(input) & INPUT_CAP_PREVIEW) {
> >     buf = xine_xmalloc(MAX_PREVIEW_SIZE);
> >     read_size = input->get_optional_data(input, buf,
> > INPUT_OPTIONAL_DATA_PREVIEW); read_size = MIN (read_size, size);
> > -   memcpy(buffer, buf, size);
> > +   memcpy(buffer, buf, read_size);
> >     free(buf);
> >   } else {
> >     return 0;
> >   }
> >   return read_size;
> > ------------------------------------------------------
> > But I think that the calling demuxer will only use
> > the first read_size bytes anyway.
>
> In the case of the real demuxer, it was using the read_size, so it would
> be for example:
>
> MAX_PREVIEW_SIZE = 4096
> size (asked for) 1024
>
> and the return len would be 4096, so:
> buf[len] = '\0' in demux_real.c would crash
> (buf[4086] is outside the buffer, of size 1024...)

Yes, that was the way it works 2 days ago. But now (after your fix)
it works as expected. With the MIN statement the
returned len should be 1024 in the above case.

Another example:

size (asked for) = 1024
Preview buffer only contains 1000 bytes (short file eg)

returned len is now 1000 but 1024 bytes are copied
into the provieded "buffer". That is not a problem but unnecassary.
The buffer should be big enough, because the calling demuxer
expects up to 1024 bytes, but the last 24 bytes only contain garbage.

cu Marco