[FFmpeg-devel,09/10] avcodec/opus_rc: Fix currently empty checks

Submitted by Andreas Rheinhardt on Sept. 18, 2019, 3:26 a.m.

Details

Message ID 20190918032607.11774-9-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Sept. 18, 2019, 3:26 a.m.
FFMAX(a, 0) is always a if a is an unsigned type. In case of opus_rc, a
is the difference of a signed and an unsigned type and the conversion to
unsigned was surely unintended.

Found via PVS-Studio (see issue #8156).

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Casting is the easy solution to this problem; one could of course use
another type, but then one would need to check whether this would not
lead to undefined or undesired behaviour elsewhere.
Given that a segfault would be the likely result of a wraparound in any
of these subtractions, I'm wondering whether the FFMAX is necessary at
all.
PVS-Studio found more bugs of this kind. Search for "will work as" on
this website: https://trac.ffmpeg.org/attachment/ticket/8156/project2019.tasks 

 libavcodec/opus_rc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/libavcodec/opus_rc.c b/libavcodec/opus_rc.c
index c432eb90c9..1c5bcefabd 100644
--- a/libavcodec/opus_rc.c
+++ b/libavcodec/opus_rc.c
@@ -391,11 +391,11 @@  void ff_opus_rc_enc_end(OpusRangeCoder *rc, uint8_t *dst, int size)
         uint8_t *rb_src, *rb_dst;
         ff_opus_rc_put_raw(rc, 0, 32 - rc->rb.cachelen);
         rb_src = rc->buf + OPUS_MAX_PACKET_SIZE + 12 - rc->rb.bytes;
-        rb_dst = dst + FFMAX(size - rc->rb.bytes, 0);
+        rb_dst = dst + FFMAX(size - (int)rc->rb.bytes, 0);
         lap = &dst[rng_bytes] - rb_dst;
         for (i = 0; i < lap; i++)
             rb_dst[i] |= rb_src[i];
-        memcpy(&rb_dst[lap], &rb_src[lap], FFMAX(rc->rb.bytes - lap, 0));
+        memcpy(&rb_dst[lap], &rb_src[lap], FFMAX((int)rc->rb.bytes - lap, 0));
     }
 }