On Tue, Sep 29, 2015 at 11:22 AM, Ronald S. Bultje <rsbul...@gmail.com> wrote: > Hi, > > On Tue, Sep 29, 2015 at 11:04 AM, Ganesh Ajjanagadde <gajja...@mit.edu> > wrote: > >> On Tue, Sep 29, 2015 at 9:24 AM, Hendrik Leppkes <h.lepp...@gmail.com> >> wrote: >> > On Sun, Sep 20, 2015 at 4:18 AM, Ganesh Ajjanagadde >> > <gajjanaga...@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 <gajjanaga...@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 >> >> >> > >> > After this patch (GCC 5.2, x86) >> > >> > libavcodec/apedec.c: In function 'do_apply_filter': >> > libavcodec/apedec.c:1284:44: warning: integer overflow in expression >> > [-Woverflow] >> > *f->adaptcoeffs = ((res & (-(1<<31))) ^ (-(1<<30))) >> >> >> Good catch - made an off by one error in my assumptions. I don't see a >> nice, clean way of dealing with -(1 << 31). >> I propose one of the following: >> 1. use INT32_MIN. >> 2. use an explicit binary/hexadecimal mask. >> 3. use e.g (-2)*(1<<30). > > > 0x80000000U is fine.
This should be ok - res will be promoted to unsigned. The bit representation being identical is not guaranteed, but in the 2's complement world it should be fine. > > Ronald > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
