[FFmpeg-devel] [PATCH] all: silence clang -Wabsolute-value for unsigned subtractions

[Ganesh Ajjanagadde]

On Sun, Aug 23, 2015 at 9:57 AM, Ganesh Ajjanagadde <gajjanag at mit.edu> wrote:
> On Sun, Aug 23, 2015 at 4:20 AM, Nicolas George <george at nsup.org> wrote:
>> Le quintidi 5 fructidor, an CCXXIII, Ganesh Ajjanagadde a écrit :
>>> >> +    return abs((int) (t1 - t2)) + abs((int) ((c1 & 0x000000ff) - (c2 & 0x000000ff))) +
>>> >> +        abs((int) (((c1 & 0x0000ff00) >> 8) - ((c2 & 0x0000ff00) >> 8))) +
>>> >> +        abs((int) (((c1 & 0x00ff0000) >> 16) - ((c2 & 0x00ff0000) >> 16)));
>>> The cast idea is incorrect as stated, though your idea is sound:
>>> int is not guaranteed to be of 32 bits on all platforms.
>>> Casting a uint32_t to an int will be bad.
>>
>> Look at the code itself: the numbers are all smaller than 768. The cast in
>> that case is completely reliable.
>
> You are right, in current use cases your solution would work.
> However, it lacks generality and is prone to mistakes:
> the code writer must reason with the bit widths as you did.

Actually, regardless this needs to be done, since abs is for ints.
Nevertheless, I think the second macro is better (without the cast to ints),
since it correctly preserves the ranges for which operation is allowed.

> Do you also agree that a macro is a better idea?
>
>>
>>> I assume this is what you had in mind with your macro idea.
>>
>> I had in mind just a local macro to make that precise part of the code more
>> readable.
>
> Such a macro would be duplicated across files.
> I just wanted to place it near the FFABS definition for removing such
> duplication,
> unless people object to this.
>
>>
>> Regards,
>>
>> --
>>   Nicolas George
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>