[FFmpeg-devel] [PATCH] avutil/mathematics: make av_gcd more robust

[Ganesh Ajjanagadde]

On Thu, Oct 29, 2015 at 4:46 PM, Michael Niedermayer
<michael at niedermayer.cc> wrote:
> On Wed, Oct 28, 2015 at 10:48:14PM -0400, Ganesh Ajjanagadde wrote:
>> This ensures that no undefined behavior is invoked, while retaining
>> identical return values in all cases and at no loss of performance
>> (identical asm on clang and gcc).
>> Essentially, this patch exchanges undefined behavior with implementation
>> defined behavior, a strict improvement.
>>
>> Rationale:
>> 1. The ideal solution is to have the return type a uint64_t. This
>> unfortunately requires an API change.
>> 2. The only pathological behavior happens if both arguments are
>> INT64_MIN, to the best of my knowledge. In such a case, the
>> implementation defined behavior is invoked in the sense that UINT64_MAX
>> is interpreted as INT64_MIN, which any reasonable implementation will
>> do. In any case, any usage where both arguments are INT64_MIN is a
>> fuzzer anyway.
>> 3. Alternatives of checking, etc require branching and lose performance
>> for no concrete gain - no client cares about av_gcd's actual value when
>> both args are INT64_MIN. Even if it did, on sane platforms (e.g all the
>> ones FFmpeg cares about), it produces a correct gcd, namely INT64_MIN.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavutil/mathematics.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> LGTM

pushed, thanks.

>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Let us carefully observe those good qualities wherein our enemies excel us
> and endeavor to excel them, by avoiding what is faulty, and imitating what
> is excellent in them. -- Plutarch
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>