[FFmpeg-devel] [PATCH] avutils/opt: fix discarded-qualifiers compiler warning

[Chad Fraleigh]

On 2/14/2021 10:16 AM, Andreas Rheinhardt wrote:
> Nuo Mi:
>> On Mon, Feb 15, 2021 at 2:08 AM Andreas Rheinhardt <
>> andreas.rheinhardt at gmail.com> wrote:
>>
>>> Nuo Mi:
>>>> ---
>>>>   libavutil/opt.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavutil/opt.c b/libavutil/opt.c
>>>> index 590146b5fb..c554e9c063 100644
>>>> --- a/libavutil/opt.c
>>>> +++ b/libavutil/opt.c
>>>> @@ -1735,7 +1735,7 @@ const AVClass *av_opt_child_class_iterate(const
>>> AVClass *parent, void **iter)
>>>>   #if FF_API_CHILD_CLASS_NEXT
>>>>   FF_DISABLE_DEPRECATION_WARNINGS
>>>>       if (parent->child_class_next) {
>>>> -        *iter = parent->child_class_next(*iter);
>>>> +        *iter = (void*)parent->child_class_next(*iter);
>>>>           return *iter;
>>>>       }
>>>>   FF_ENABLE_DEPRECATION_WARNINGS
>>>>
>>> This doesn't look like a fix; you just silenced a warning.
>>>
>> Yes, but this the only way if we do not change the function signature.

Suppressing a warning that something isn't right (i.e. that maybe the 
function signature *should* be changed) probably isn't ideal, except as 
a last resort. The fact that the documentation literally says to do the 
cast this patch is trying to do only compounds the problem.

I can understand wanting to use 'void *' for the context, in order to 
hide the underlying implementation. But since the documentation has 
already implied an implementation, hiding it may be moot. However, in 
any implementation, it really should be 'const void **', as iteration 
should have no internal side effects.

This leads back to the original statement of not covering up the problem 
(i.e. no nothing now -- but not just because it's deprecated) and 
encourage a signature change to be done (at an appropriate API breakage 
point).


> There is another way: Do nothing. The code in question is deprecated and
> will therefore eventually be removed.