On Mon, Feb 15, 2021 at 1:39 AM Carl Eugen Hoyos <ceffm...@gmail.com> wrote:
> 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. > You are right, the count is unsigned int, but https://github.com/FFmpeg/FFmpeg/blob/21346672270ae723aa774a9c8b0749954a75b3df/libavcodec/movtextenc.c#L112 tells us the value never > 16 bits. so the multiple will never overflow(if we check "count <= UINT16_MAX" somewhere). > > > > 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". _______________________________________________ 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".
