| Message ID | 20190915200120.3784-1-andreas.rheinhardt@gmail.com |
|---|---|
| State | Accepted |
| Commit | 3ab488a5407f833ecc66e8fa4c537dc4852db720 |
| Headers | show |
LGTM On 9/15/19, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote: > ttaenc contained (1 << unary) - 1 as an argument for a function > expecting an unsigned int. unary can be as big as 31 in this case. > The type of the shift and the whole expression is int, because 1 fits > into an integer, so that the behaviour is undefined if unary == 31 > as the result of the shift can't be represented in an int §. Subtraction > by 1 (which makes the result of the whole expression representable in > an int) doesn't change that this is undefined (it usually leads to > signed integer overflow which is undefined, too). > > The solution is simple: Make 1 unsigned to change the type of the > whole expression to unsigned int (as the function expects anyway). > > Fixes ticket #8153. > > §: This of course presupposes the common int range of -2^31..2^31-1 > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> > --- > libavcodec/ttaenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/libavcodec/ttaenc.c b/libavcodec/ttaenc.c > index 3cc54d78c5..08a0d0483a 100644 > --- a/libavcodec/ttaenc.c > +++ b/libavcodec/ttaenc.c > @@ -164,7 +164,7 @@ pkt_alloc: > put_bits(&pb, 31, 0x7FFFFFFF); > unary -= 31; > } else { > - put_bits(&pb, unary, (1 << unary) - 1); > + put_bits(&pb, unary, (1U << unary) - 1); > unary = 0; > } > } while (unary); > -- > 2.21.0 > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Mon, Sep 16, 2019 at 10:16:36AM +0200, Paul B Mahol wrote:
> LGTM
will apply
thx
[...]
diff --git a/libavcodec/ttaenc.c b/libavcodec/ttaenc.c index 3cc54d78c5..08a0d0483a 100644 --- a/libavcodec/ttaenc.c +++ b/libavcodec/ttaenc.c @@ -164,7 +164,7 @@ pkt_alloc: put_bits(&pb, 31, 0x7FFFFFFF); unary -= 31; } else { - put_bits(&pb, unary, (1 << unary) - 1); + put_bits(&pb, unary, (1U << unary) - 1); unary = 0; } } while (unary);
ttaenc contained (1 << unary) - 1 as an argument for a function expecting an unsigned int. unary can be as big as 31 in this case. The type of the shift and the whole expression is int, because 1 fits into an integer, so that the behaviour is undefined if unary == 31 as the result of the shift can't be represented in an int §. Subtraction by 1 (which makes the result of the whole expression representable in an int) doesn't change that this is undefined (it usually leads to signed integer overflow which is undefined, too). The solution is simple: Make 1 unsigned to change the type of the whole expression to unsigned int (as the function expects anyway). Fixes ticket #8153. §: This of course presupposes the common int range of -2^31..2^31-1 Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavcodec/ttaenc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)