[FFmpeg-devel] [PATCH] fix warning in libavutil/fifo.c

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Sun, Jan 25, 2009 at 03:58:48PM +0100, Diego Biurrun wrote:
>> On Tue, Jan 13, 2009 at 12:52:01PM -0800, Baptiste Coudurier wrote:
>> > 
>> > Michael Niedermayer wrote:
>> > > On Mon, Jan 12, 2009 at 10:33:12PM +0100, Diego Biurrun wrote:
>> > >> On Mon, Jan 12, 2009 at 09:13:58PM +0100, Michael Niedermayer wrote:
>> > >>> On Sun, Jan 11, 2009 at 10:17:18PM +0100, Diego Biurrun wrote:
>> > >>>> Here is a patch to fix
>> > >>>>
>> > >>>> fifo.c:79: warning: cast discards qualifiers from pointer target type
>> > >>>>
>> > >>>> --- libavutil/fifo.c	(revision 16549)
>> > >>>> +++ libavutil/fifo.c	(working copy)
>> > >>>> @@ -74,7 +74,7 @@
>> > >>>>  
>> > >>>> -void av_fifo_write(AVFifoBuffer *f, const uint8_t *buf, int size)
>> > >>>> +void av_fifo_write(AVFifoBuffer *f, uint8_t *buf, int size)
>> > >>> I belive av_fifo_write does not change the content of buf thus const
>> > >>> is correct
>> > >> Umm, I beg to differ. av_fifo_write is just a wrapper for
>> > >> av_fifo_generic_write, which does modify buf, hence the warning.
>> > > 
>> > > where does it modify the _CONTENT_OF_ buf?
>> > > 
>> > > It does not do this anywhere in the case where it is called through
>> > > av_fifo_write thus the const is correct.
>> > > av_fifo_write can be used with const arrays, if you remove the cont in the
>> > > parameter of av_fifo_write() you will have a warning for every use of
>> > > av_fifo_write() with a const argument, i doubt you prefer this ...
>> > 
>> > Would changing the prototype of 'func' to take const void * fix it ?
>> 
>> As in $attached?
>> 
>> Diego
>
>> Index: libavutil/fifo.c
>> ===================================================================
>> --- libavutil/fifo.c	(revision 16768)
>> +++ libavutil/fifo.c	(working copy)
>> @@ -76,10 +76,10 @@
>>  
>>  void av_fifo_write(AVFifoBuffer *f, const uint8_t *buf, int size)
>>  {
>> -    av_fifo_generic_write(f, (void *)buf, size, NULL);
>> +    av_fifo_generic_write(f, (const void *)buf, size, NULL);
>>  }
>>  
>> -int av_fifo_generic_write(AVFifoBuffer *f, void *src, int size, int (*func)(void*, void*, int))
>> +int av_fifo_generic_write(AVFifoBuffer *f, const void *src, int size, int (*func)(const void*, void*, int))
>
> this breaks the API and causes undefined behavior, src is not a const
> in av_fifo_generic_write(), it can be a struct that may contain a pointer
> for example that needs updating.
>
> To summarize this
> av_fifo_generic_write MUST NOT have a const src
> av_fifo_write SHOULD have a const src
> gcc prints a warning if you by whatever means change a const to a non
> const with the current flags.
>
> solutions are
> * close your eyes
> * change the flags
> * change gcc
> * filter the gcc output
>
> please dont change correct code due to incorrect warnings (or at least
> idiotic warnings)

av_fifo_write() is slated for removal, so is this really important?

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