[FFmpeg-devel] [PATCH] avcodec/apedec: fix undefined left shifts of negative numbers

Ganesh Ajjanagadde gajjanag at mit.edu
Tue Sep 29 14:08:54 CEST 2015


On Tue, Sep 29, 2015 at 4:11 AM, Paul B Mahol <onemda at gmail.com> wrote:
> On 9/25/15, Ganesh Ajjanagadde <gajjanagadde at gmail.com> wrote:
>> On Sat, Sep 19, 2015 at 10:18 PM, Ganesh Ajjanagadde
>> <gajjanagadde at gmail.com> wrote:
>>> This fixes -Wshift-negative-value reported with clang 3.7+, e.g
>>> http://fate.ffmpeg.org/log.cgi?time=20150919172459&log=compile&slot=x86_64-darwin-clang-polly-notiling-3.7.
>>> Note that the patch crucially depends on int >= 32 bits,
>>> an assumption made in many places in the codebase.
>>>
>>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>>> ---
>>>  libavcodec/apedec.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
>>> index 5536e0f..7b34d26 100644
>>> --- a/libavcodec/apedec.c
>>> +++ b/libavcodec/apedec.c
>>> @@ -1281,7 +1281,7 @@ static void do_apply_filter(APEContext *ctx, int
>>> version, APEFilter *f,
>>>              /* Update the adaption coefficients */
>>>              absres = FFABS(res);
>>>              if (absres)
>>> -                *f->adaptcoeffs = ((res & (-1<<31)) ^ (-1<<30)) >>
>>> +                *f->adaptcoeffs = ((res & (-(1<<31))) ^ (-(1<<30))) >>
>>>                                    (25 + (absres <= f->avg*3) + (absres <=
>>> f->avg*4/3));
>>>              else
>>>                  *f->adaptcoeffs = 0;
>>> --
>>> 2.5.2
>>>
>>
>> ping
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>
> I guess its fine if fate passes.

Can't confirm that on my end, since I don't know if my fate runs a
test for this. Can someone tell me an easy way to check if make fate
tests a particular code or note for future reference?

On the other hand, note that -(1 << n) and (-1 << n) are identical at
least on GCC and clang, so I think this should be fine.

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


More information about the ffmpeg-devel mailing list