Hi, On Fri, Oct 16, 2015 at 8:18 PM, Ganesh Ajjanagadde <gajja...@mit.edu> wrote:
> On Fri, Oct 16, 2015 at 8:05 PM, Ronald S. Bultje <rsbul...@gmail.com> > wrote: > > Hi, > > > > On Fri, Oct 16, 2015 at 5:48 PM, Ganesh Ajjanagadde <gajja...@mit.edu> > > wrote: > > > >> On Fri, Oct 16, 2015 at 5:45 PM, Hendrik Leppkes <h.lepp...@gmail.com> > >> wrote: > >> > On Fri, Oct 16, 2015 at 11:39 PM, Ganesh Ajjanagadde > >> > <gajjanaga...@gmail.com> wrote: > >> >> On Wed, Oct 14, 2015 at 10:05 PM, Ganesh Ajjanagadde > >> >> <gajjanaga...@gmail.com> wrote: > >> >>> This patch results in identical behavior of movenc, and suppresses > >> -Wstrict-overflow > >> >>> warnings observed in GCC 5.2: > >> >>> > >> > http://fate.ffmpeg.org/log.cgi?time=20150926231053&log=compile&slot=x86_64-archlinux-gcc-threads-misc > >> , > >> >>> "warning: assuming signed overflow does not occur when assuming that > >> (X - c) > X is always false [-Wstrict-overflow]" > >> >>> I have manually checked that all usages are safe, and overflow > >> possibility does > >> >>> not exist with this expression rewrite. > >> >>> > >> >>> Some expressed concern over readability loss, hence a comment is > added. > >> >>> This is the simplest way to suppress this warning. > >> >>> > >> >>> Signed-off-by: Ganesh Ajjanagadde <gajjanaga...@gmail.com> > >> >>> --- > >> >>> libavformat/movenc.c | 4 +++- > >> >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >> >>> > >> >>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c > >> >>> index 5115585..ff997f2 100644 > >> >>> --- a/libavformat/movenc.c > >> >>> +++ b/libavformat/movenc.c > >> >>> @@ -854,7 +854,9 @@ static int get_cluster_duration(MOVTrack *track, > >> int cluster_idx) > >> >>> { > >> >>> int64_t next_dts; > >> >>> > >> >>> - if (cluster_idx >= track->entry) > >> >>> + /* GCC 5.2 wants to "optimize" cluster_idx >= track->entry to > the > >> below > >> >>> + * expression. We actually mean cluster_idx >= track->entry. */ > >> >>> + if (cluster_idx - track->entry >= 0) > >> >>> return 0; > >> >>> > >> >>> if (cluster_idx + 1 == track->entry) > >> >>> -- > >> >>> 2.6.1 > >> >>> > >> >> > >> >> ping, is this solution acceptable? Note that I will get rid of the > >> >> extra * on the second line of the comment. > >> > > >> > Not a fan myself of any such hacks to get compilers to shut up. Is GCC > >> > doing something clearly wrong here, and the false warning should be > >> > reported? > >> > >> I gave an (extremely detailed) explanation of what was going on > >> earlier in the thread: > >> https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180976.html. > >> The punch line is that it is not clearly wrong, and alternatives for > >> warning suppression are less clean. > > > > > > And I responded in the same thread: > > https://ffmpeg.org/pipermail/ffmpeg-devel/2015-October/180977.html > > > > I'm still not a fan of this either. I don't think clang should be doing > > what it's doing here. > > Neither am I a fan of this whole business. I try to make do with what > is feasible, clean, and easy to revert at a future stage. > > My report was with gcc, have not checked clang. Funny that both of > them try to do this. > > What alternative do you propose? Just keep the warning as it is, and > forget suppressing it? > > Also, I would like to point out the following. Just see the following > lines in avcodec/aacdec_template.c: > /* This assignment is to silence a GCC warning about the variable > being used > * uninitialized when in fact it always is. > */ > pulse.num_pulse = 0; > > I have tested, and confirmed that only compilers strictly prior to GCC > 4.6 actually complain. I showed this to Michael, and he did not feel > like removing this ridiculous bit of code. It was in light of this > that I crafted a patch with a comment, and thought people would be > fine with it. Sometimes these things are just based on personal opinions. :( I personally wouldn't mind removing the assignment if recent gcc versions don't complain anymore. Ronald _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
