Am So., 14. Feb. 2021 um 18:15 Uhr schrieb Nuo Mi <nuomi2...@gmail.com>: > > On Mon, Feb 15, 2021 at 1:05 AM Carl Eugen Hoyos <ceffm...@gmail.com> wrote: > > > Am So., 14. Feb. 2021 um 17:30 Uhr schrieb Nuo Mi <nuomi2...@gmail.com>: > > > > > > On Sun, Feb 14, 2021 at 6:35 PM Anton Khirnov <an...@khirnov.net> wrote: > > > > > > > Quoting Nuo Mi (2021-02-14 07:27:39) > > > > > CC libavcodec/mpegaudiodec_common.o > > > > > libavcodec/movtextenc.c: In function ‘mov_text_style_start’: > > > > > libavcodec/movtextenc.c:358:26: warning: comparison is always false > > due > > > > to limited range of data type [-Wtype-limits] > > > > > 358 | if (s->count + 1 > SIZE_MAX / > > > > sizeof(*s->style_attributes) || > > > > > --- > > > > > libavcodec/movtextenc.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c > > > > > index 1bef21e0b9..cd0e43a79b 100644 > > > > > --- a/libavcodec/movtextenc.c > > > > > +++ b/libavcodec/movtextenc.c > > > > > @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext > > *s) > > > > > StyleBox *tmp; > > > > > > > > > > // last style != defaults, end the style entry and start a > > new > > > > one > > > > > - if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) > > || > > > > > + if ((s->count + 1) * sizeof(*s->style_attributes) > > > SIZE_MAX || > > > > > > > > What guarantees the multiplication does not overflow? > > > > > > > Hi Anton, > > > Thanks for the review. > > > It never overflows on 64bits system. > > > > But not every system is a 64bit system > > > > > This why the compiler has warning on > > > my 64 bits system. > > > > Yes, we also see the warning on 64bit systems. > > > > > If you check > > > > > https://github.com/FFmpeg/FFmpeg/blob/21346672270ae723aa774a9c8b0749954a75b3df/libavcodec/movtextenc.c#L110 > > > s->count * sizeof(*s->style_attributes) never > 32 bits.
This is not correct afaict: The relevant line is 369 not 110, count is of type unsigned and if you multiply it with something >1, it can overflow. > > > I can add some check for s->count check based on the code if you > > > are preferred. > > > > Isn't this exactly the check that is already there? > No, we only check the overflow, the overflow depends on the machine, Yes. > it never overflow on a 64 bits system. Yes. > And the limitation is not from the spec. Yes. > From this codec( > https://github.com/FFmpeg/FFmpeg/blob/21346672270ae723aa774a9c8b0749954a75b3df/libavcodec/movtextenc.c#L112). > s->count never > 16 bits. > Instead of depends on system check, I think check s->count <= UINT16_MAX is > more reasonable. The rest of your comments do not look correct to me but I may miss something. I suspect that if it were simple to silence this warning it would be already be silenced. Carl Eugen _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
