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

[Måns Rullgård]

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.

-- 
M?ns Rullg?rd
mans at mansr.com