[FFmpeg-devel] [PATCHv2] avformat/movenc: suppress -Wstrict-overflow warnings

[Ronald S. Bultje]

Hi,

On Fri, Oct 16, 2015 at 5:48 PM, Ganesh Ajjanagadde <gajjanag at mit.edu>
wrote:

> On Fri, Oct 16, 2015 at 5:45 PM, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> > On Fri, Oct 16, 2015 at 11:39 PM, Ganesh Ajjanagadde
> > <gajjanagadde at gmail.com> wrote:
> >> On Wed, Oct 14, 2015 at 10:05 PM, Ganesh Ajjanagadde
> >> <gajjanagadde at 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 <gajjanagadde at 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.

Ronald