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.
>
> Ronald
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
