diff mbox series

[FFmpeg-devel,V3,3/5] libavformat/protocols.c: fix build warning for [-Wdiscarded-qualifiers]

Message ID 20210226083727.2042-3-yejun.guo@intel.com
State New
Headers show
Series [FFmpeg-devel,V3,1/5] libavdevice/v4l2.c: fix build warning for [-Wformat-truncation=]
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Guo, Yejun Feb. 26, 2021, 8:37 a.m. UTC
src/libavformat/protocols.c: In function ‘avio_enum_protocols’:
src/libavformat/protocols.c:116:7: warning: assignment discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
     p = p ? p + 1 : url_protocols;
       ^

Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
---
 libavformat/protocols.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul B Mahol Feb. 26, 2021, 11:36 a.m. UTC | #1
look at same/similar patches like yours that have been already rejected.

On Fri, Feb 26, 2021 at 9:48 AM Guo, Yejun <yejun.guo@intel.com> wrote:

> src/libavformat/protocols.c: In function ‘avio_enum_protocols’:
> src/libavformat/protocols.c:116:7: warning: assignment discards ‘const’
> qualifier from pointer target type [-Wdiscarded-qualifiers]
>      p = p ? p + 1 : url_protocols;
>        ^
>
> Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> ---
>  libavformat/protocols.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/protocols.c b/libavformat/protocols.c
> index 7df18fbb3b..86cde84a31 100644
> --- a/libavformat/protocols.c
> +++ b/libavformat/protocols.c
> @@ -113,7 +113,7 @@ const char *avio_enum_protocols(void **opaque, int
> output)
>  {
>      const URLProtocol **p = *opaque;
>
> -    p = p ? p + 1 : url_protocols;
> +    p = p ? p + 1 : (const URLProtocol **)url_protocols;
>      *opaque = p;
>      if (!*p) {
>          *opaque = NULL;
> --
> 2.17.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Guo, Yejun Feb. 27, 2021, 8:14 a.m. UTC | #2
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Paul B
> Mahol
> Sent: 2021年2月26日 19:37
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Guo, Yejun <yejun.guo@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH V3 3/5] libavformat/protocols.c: fix build
> warning for [-Wdiscarded-qualifiers]
> 
> look at same/similar patches like yours that have been already rejected.

thanks for the info.

My motivation to fix the build warnings is that: I have several patches caught
by patchwork with build warnings that I missed. I'd like to avoid it. And the best method,
IMO, is to add '-Wall -Werror' for the build option. Such option can also help to find
potential issues in the code. This is the reason that I plan to fix the build warnings
one by one together with community.

Looks that there are build warnings hard to have a nice fix, I'll reconsider my plan, thanks.

> 
> On Fri, Feb 26, 2021 at 9:48 AM Guo, Yejun <yejun.guo@intel.com> wrote:
> 
> > src/libavformat/protocols.c: In function ‘avio_enum_protocols’:
> > src/libavformat/protocols.c:116:7: warning: assignment discards ‘const’
> > qualifier from pointer target type [-Wdiscarded-qualifiers]
> >      p = p ? p + 1 : url_protocols;
> >        ^
> >
> > Signed-off-by: Guo, Yejun <yejun.guo@intel.com>
> > ---
> >  libavformat/protocols.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/protocols.c b/libavformat/protocols.c index
> > 7df18fbb3b..86cde84a31 100644
> > --- a/libavformat/protocols.c
> > +++ b/libavformat/protocols.c
> > @@ -113,7 +113,7 @@ const char *avio_enum_protocols(void **opaque,
> int
> > output)
> >  {
> >      const URLProtocol **p = *opaque;
> >
> > -    p = p ? p + 1 : url_protocols;
> > +    p = p ? p + 1 : (const URLProtocol **)url_protocols;
> >      *opaque = p;
> >      if (!*p) {
> >          *opaque = NULL;
> > --
> > 2.17.1
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Reimar Döffinger Feb. 27, 2021, 2:08 p.m. UTC | #3
> On 27 Feb 2021, at 09:14, Guo, Yejun <yejun.guo@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Paul B
>> Mahol
>> Sent: 2021年2月26日 19:37
>> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
>> Cc: Guo, Yejun <yejun.guo@intel.com>
>> Subject: Re: [FFmpeg-devel] [PATCH V3 3/5] libavformat/protocols.c: fix build
>> warning for [-Wdiscarded-qualifiers]
>> 
>> look at same/similar patches like yours that have been already rejected.
> 
> thanks for the info.
> 
> My motivation to fix the build warnings is that: I have several patches caught
> by patchwork with build warnings that I missed. I'd like to avoid it. And the best method,
> IMO, is to add '-Wall -Werror' for the build option.

I think that is a bad idea, compilers add new warnings all the time, many of them
unreliable or of questionable quality/usefulness.
What you end up with using -Werror is a project that constantly fails to build and gets
a reputation for low quality.
However there may well be specific, reliable warning types for which we
should add an error specifically, as we already do now for e.g.
-Werror=implicit-function-declaration
There might also be other options to improve things and ensure relevant
warnings are visible and addressed, but I’ve yet to
see a -Werror have convincing effects (for example one effect it usually
has is that people have to stick to “supported” compiler versions to avoid
constant issues with build failures, reducing testing and actually decreasing
code quality instead of increasing it).
Turning it around, using -Werror successfully would mean testing with all
compiler versions regularly including pre-release versions and ensuring
any issues are fixed immediately. Which I don’t think is an approach
that would work for FFmpeg.

> Such option can also help to find
> potential issues in the code. This is the reason that I plan to fix the build warnings
> one by one together with community.

I believe many are likely to be fixable in a way that most developers
would be quite happy with, even consider it an improvement beyond
just the warning/bug fix.
It’ll just take a lot more effort to find those types of fixes.
After all if it was that simple there could just be an option for
the compiler to fix them itself, the reason there isn’t is because
it would likely end very badly.

Best regards,
Reimar
Guo, Yejun Feb. 28, 2021, 8:45 a.m. UTC | #4
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Reimar
> D?ffinger
> Sent: 2021年2月27日 22:09
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH V3 3/5] libavformat/protocols.c: fix build
> warning for [-Wdiscarded-qualifiers]
> 
> 
> 
> > On 27 Feb 2021, at 09:14, Guo, Yejun <yejun.guo@intel.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> >> Paul B Mahol
> >> Sent: 2021年2月26日 19:37
> >> To: FFmpeg development discussions and patches
> >> <ffmpeg-devel@ffmpeg.org>
> >> Cc: Guo, Yejun <yejun.guo@intel.com>
> >> Subject: Re: [FFmpeg-devel] [PATCH V3 3/5] libavformat/protocols.c:
> >> fix build warning for [-Wdiscarded-qualifiers]
> >>
> >> look at same/similar patches like yours that have been already rejected.
> >
> > thanks for the info.
> >
> > My motivation to fix the build warnings is that: I have several
> > patches caught by patchwork with build warnings that I missed. I'd
> > like to avoid it. And the best method, IMO, is to add '-Wall -Werror' for the
> build option.
> 
> I think that is a bad idea, compilers add new warnings all the time, many of
> them unreliable or of questionable quality/usefulness.
> What you end up with using -Werror is a project that constantly fails to build
> and gets a reputation for low quality.
> However there may well be specific, reliable warning types for which we
> should add an error specifically, as we already do now for e.g.
> -Werror=implicit-function-declaration
> There might also be other options to improve things and ensure relevant
> warnings are visible and addressed, but I’ve yet to see a -Werror have
> convincing effects (for example one effect it usually has is that people have to
> stick to “supported” compiler versions to avoid constant issues with build
> failures, reducing testing and actually decreasing code quality instead of
> increasing it).
> Turning it around, using -Werror successfully would mean testing with all
> compiler versions regularly including pre-release versions and ensuring any
> issues are fixed immediately. Which I don’t think is an approach that would
> work for FFmpeg.
> 
> > Such option can also help to find
> > potential issues in the code. This is the reason that I plan to fix
> > the build warnings one by one together with community.
> 
> I believe many are likely to be fixable in a way that most developers would be
> quite happy with, even consider it an improvement beyond just the
> warning/bug fix.
> It’ll just take a lot more effort to find those types of fixes.
> After all if it was that simple there could just be an option for the compiler to
> fix them itself, the reason there isn’t is because it would likely end very badly.
> 
> Best regards,
> Reimar

thanks for your nice explanation, thank you.
diff mbox series

Patch

diff --git a/libavformat/protocols.c b/libavformat/protocols.c
index 7df18fbb3b..86cde84a31 100644
--- a/libavformat/protocols.c
+++ b/libavformat/protocols.c
@@ -113,7 +113,7 @@  const char *avio_enum_protocols(void **opaque, int output)
 {
     const URLProtocol **p = *opaque;
 
-    p = p ? p + 1 : url_protocols;
+    p = p ? p + 1 : (const URLProtocol **)url_protocols;
     *opaque = p;
     if (!*p) {
         *opaque = NULL;