[FFmpeg-devel] [PATCH] avcodec/wavpack: check for overflow

[Paul B Mahol]

On 7/2/13, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Mon, Jun 17, 2013 at 05:49:47PM +0200, Michael Niedermayer wrote:
>> On Mon, Jun 17, 2013 at 10:54:28AM +0000, Paul B Mahol wrote:
>> > On 6/15/13, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > > Fix integer overflow in fate
>> > >
>> > > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> > > ---
>> > >  libavcodec/wavpack.c |   10 ++++++++--
>> > >  1 file changed, 8 insertions(+), 2 deletions(-)
>> > >
>> > > diff --git a/libavcodec/wavpack.c b/libavcodec/wavpack.c
>> > > index 47f598a..dd273f7 100644
>> > > --- a/libavcodec/wavpack.c
>> > > +++ b/libavcodec/wavpack.c
>> > > @@ -581,8 +581,14 @@ static inline int
>> > > wv_unpack_stereo(WavpackFrameContext
>> > > *s, GetBitContext *gb,
>> > >                      L2 = L + ((s->decorr[i].weightA * (int64_t)A +
>> > > 512) >>
>> > > 10);
>> > >                      R2 = R + ((s->decorr[i].weightB * (int64_t)B +
>> > > 512) >>
>> > > 10);
>> > >                  } else {
>> > > -                    L2 = L + ((s->decorr[i].weightA * A + 512) >>
>> > > 10);
>> > > -                    R2 = R + ((s->decorr[i].weightB * B + 512) >>
>> > > 10);
>> > > +                    int64_t Lt = s->decorr[i].weightA * (int64_t)A +
>> > > 512;
>> > > +                    int64_t Rt = s->decorr[i].weightB * (int64_t)B +
>> > > 512;
>> > > +                    if ((int32_t)Lt != Lt || (int32_t)Rt != Rt) {
>> > > +                        av_log(s->avctx, AV_LOG_ERROR, "sample
>> > > overflow %d
>> >
>> > This looks extremly ugly.
>>
>> Iam quite aware of that, which is part of the reason why i posted this
>> (aka do you know a less ugly solution?)
>
> ping
> anyone has a better solution ?
> do any objections to the original patch remain ?

Yes, i see no point to apply this patch as is.

>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> If a bugfix only changes things apparently unrelated to the bug with no
> further explanation, that is a good sign that the bugfix is wrong.
>