[FFmpeg-devel] lavf/protocols: Fix compile warning of discarding 'const' qualifier from pointer target type

Signed-off-by: Linjie Fu <linjie.fu@intel.com>
---
 libavformat/protocols.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Linjie Fu:
> Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> ---
>  libavformat/protocols.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> index f1b8eab..cb19f77 100644
> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -95,9 +95,9 @@ const AVClass *ff_urlcontext_child_class_next(const AVClass *prev)
>  
>  const char *avio_enum_protocols(void **opaque, int output)
>  {
> -    const URLProtocol **p = *opaque;
> +    URLProtocol **p = *opaque;
>  
> -    p = p ? p + 1 : url_protocols;
> +    p = p ? p + 1 : (URLProtocol **)url_protocols;
>      *opaque = p;
>      if (!*p) {
>          *opaque = NULL;
> 
1. The actual problem with this code is not that the URLProtocols are
const (which they are), but that the pointers in the url_protocols array
are constant pointers (to const URLProtocol); yet the way things are
right now the user gets a non-const pointer to these const pointers (to
const URLProtocol).

2. It follows from this that changing the type of p in the way you did
it is actually unnecessary (if one wanted to solve the problem by
casting const away, you could cast url_protocols to (const URLProtocol
**) (from const URLProtocol * const *)) and a step in the wrong
direction (given that the underlying URLProtocol is const).

3. Casting const away is generally a sign of bad design and this is no
exception. The signature is wrong, the opaque should be const void**, so
that the user never gets a chance to get a pointer to nonconst to
something that is actually const.

4. See [1] for an approach that fixes the underlying issue.

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248675.html

> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Andreas Rheinhardt
> Sent: Wednesday, March 18, 2020 12:52
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavf/protocols: Fix compile warning of
> discarding 'const' qualifier from pointer target type
> 
> Linjie Fu:
> > Signed-off-by: Linjie Fu <linjie.fu@intel.com>
> > ---
> >  libavformat/protocols.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> > index f1b8eab..cb19f77 100644
> > --- a/libavformat/protocols.c
> > +++ b/libavformat/protocols.c
> > @@ -95,9 +95,9 @@ const AVClass *ff_urlcontext_child_class_next(const
> AVClass *prev)
> >
> >  const char *avio_enum_protocols(void **opaque, int output)
> >  {
> > -    const URLProtocol **p = *opaque;
> > +    URLProtocol **p = *opaque;
> >
> > -    p = p ? p + 1 : url_protocols;
> > +    p = p ? p + 1 : (URLProtocol **)url_protocols;
> >      *opaque = p;
> >      if (!*p) {
> >          *opaque = NULL;
> >
> 1. The actual problem with this code is not that the URLProtocols are
> const (which they are), but that the pointers in the url_protocols array
> are constant pointers (to const URLProtocol); yet the way things are
> right now the user gets a non-const pointer to these const pointers (to
> const URLProtocol).

Yes, a "const" is needed for "void ** opaque" since it's a pointer to a pointer
to a const type.

> 2. It follows from this that changing the type of p in the way you did
> it is actually unnecessary (if one wanted to solve the problem by
> casting const away, you could cast url_protocols to (const URLProtocol
> **) (from const URLProtocol * const *)) and a step in the wrong
> direction (given that the underlying URLProtocol is const).

You are right.

> 3. Casting const away is generally a sign of bad design and this is no
> exception. The signature is wrong, the opaque should be const void**, so
> that the user never gets a chance to get a pointer to nonconst to
> something that is actually const.
> 
> 4. See [1] for an approach that fixes the underlying issue.
> 
> - Andreas
> 
> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-August/248675.html

Thanks for the elaborations, the fix in [1] is more reasonable.
However got one concern while verifying the patch, replied under your thread.

- Linjie