James Almer: > On 5/25/2020 11:07 AM, Andreas Rheinhardt wrote: >> If adding two ints overflows, it doesn't matter whether the result will >> be stored in an unsigned or not; and checking afterwards does not make it >> retroactively defined. >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> >> --- >> libavformat/aviobuf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c >> index eb0387bdf7..33c2d6f037 100644 >> --- a/libavformat/aviobuf.c >> +++ b/libavformat/aviobuf.c >> @@ -1275,7 +1275,7 @@ static int dyn_buf_write(void *opaque, uint8_t *buf, >> int buf_size) >> unsigned new_size, new_allocated_size; >> >> /* reallocate buffer if needed */ >> - new_size = d->pos + buf_size; >> + new_size = (unsigned)d->pos + buf_size; >> new_allocated_size = d->allocated_size; >> if (new_size < d->pos || new_size > INT_MAX/2) >> return -1; > > You could instead do > > if (d->pos > INT_MAX / 2 - buf_size) > return -1; > new_size = d->pos + buf_size; > > Should also work fine with the changes in 3/5 adapted to it.
This would drop the check for whether buf_size is negative. While I am not opposed to this (such a check should be made generically, so that every AVIOContext.write_packet() can rely on buf_size being >= 0 (0 could probably be also dealt with generically)), I intended to do this only later (in a future patchset). This implied for my/the current version that one could drop the check for new_size < d->pos. And with this change your proposal loses its advantage. (Also notice that if buf_size is negative, INT_MAX / 2 - buf_size might overflow and INT_MAX - buf_size does overflow.) - Andreas _______________________________________________ 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".
