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. > > > 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? > Hi Carl, thanks for the review. No, we only check the overflow, the overflow depends on the machine, it never overflow on a 64 bits system. And the limitation is not from the spec. 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. > 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". _______________________________________________ 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".