[FFmpeg-cvslog] r8900 - trunk/libavcodec/mpegaudio_parser.c

[Aurelien Jacobs]

On Sun, 06 May 2007 10:31:53 +0100
M?ns Rullg?rd <mans at mansr.com> wrote:

> Aurelien Jacobs <aurel at gnuage.org> writes:
> 
> > On Sat, 05 May 2007 23:54:22 +0100
> > M?ns Rullg?rd <mans at mansr.com> wrote:
> >
> >> aurel <subversion at mplayerhq.hu> writes:
> >> 
> >> > Author: aurel
> >> > Date: Sat May  5 20:17:03 2007
> >> > New Revision: 8900
> >> >
> >> > Log:
> >> > avoid a 'discards qualifiers from pointe' warning
> >> >
> >> > Modified:
> >> >    trunk/libavcodec/mpegaudio_parser.c
> >> >
> >> > Modified: trunk/libavcodec/mpegaudio_parser.c
> >> > ==============================================================================
> >> > --- trunk/libavcodec/mpegaudio_parser.c	(original)
> >> > +++ trunk/libavcodec/mpegaudio_parser.c	Sat May  5 20:17:03 2007
> >> > @@ -186,7 +186,7 @@ static int mpegaudio_parse(AVCodecParser
> >> >          if(s->frame_size > 0 && buf_ptr - buf == s->inbuf_ptr - s->inbuf
> >> >             && buf_size + buf_ptr - buf >= s->frame_size){
> >> >              if(s->header_count > 0){
> >> > -                *poutbuf = buf;
> >> > +                *poutbuf = (uint8_t *)buf;
> >> 
> >> This is wrong.  Never, *ever* remove a const qualifier.  The data
> >> pointed to might actually be read-only.  The correct solution is to
> >> make the assigned to variable also a pointer to const.
> >
> > Ok. That was my feeling when adding this, but I still did it this way
> > because that's the way it was already done in most parsers.
> > Now I think that it's safer the other way (ie. casting a non-const to
> > a const), but I'm not sure it can't cause trouble ?
> 
> A pointer to non-const data can safely be assigned to a pointer to
> const.  All a pointer to const means is that you can't modify the data
> through that pointer.  If you look at the headers, you'll find that
> functions like strcmp() and memcpy() take pointer to const arguments.
> 
> > If it's safe, then something like the attached patch should be applied ?
> > (I will look at it a bit deeper tomorrow. It's getting late.)
> 
> >  static int jpeg_parse(AVCodecParserContext *s,
> >                             AVCodecContext *avctx,
> > -                           uint8_t **poutbuf, int *poutbuf_size,
> > +                           const uint8_t **poutbuf, int *poutbuf_size,
> >                             const uint8_t *buf, int buf_size)
> 
> This looks sensible.  I don't have time right now for a careful
> scrutiny of the patch.

I checked my patch a bit more carefully and looks good to me.
At least it shouldn't be worse than previous situation, so I just
applied it.

Aurel