| 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=] | expand |
| 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 |
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".
> -----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".
> 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
> -----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 --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;
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(-)