From 92e483f8ed70d88d4f64337f65bae212502735d4 Mon Sep 17 00:00:00 2001
From: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
Date: Sun, 1 Nov 2015 10:43:56 -0500
Subject: [PATCH] all: use FFDIFFSIGN to resolve possible undefined behavior in
comparators
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
FFDIFFSIGN was created explicitly for this purpose, since the common
return a - b idiom is unsafe regarding overflow on signed integers. It
optimizes to branchless code on common compilers.
FFDIFFSIGN also has the subjective benefit of being easier to read due
to lack of ternary operators.
Tested with FATE.
Things not covered by this are unsigned integers, for which overflows
are well defined, and also places where overflow is clearly impossible,
e.g an instance where the a - b was being done on 24 bit values.
Reviewed-by: Michael Niedermayer <michael@niedermayer.cc>
Reviewed-by: Clément Bœsch <u@pkh.me>
Signed-off-by: Ganesh Ajjanagadde <gajjanagadde@gmail.com>
---
cmdutils.c | 2 +-
cmdutils_opencl.c | 4 +++-
ffmpeg.c | 3 +--
libavfilter/f_sendcmd.c | 6 +-----
libavfilter/vf_deshake.c | 3 +--
libavfilter/vf_palettegen.c | 2 +-
libavfilter/vf_removegrain.c | 5 ++---
libavformat/subtitles.c | 9 +++------
8 files changed, 13 insertions(+), 21 deletions(-)
diff --git a/cmdutils.c b/cmdutils.c
index e3e989184c6..41daa9565d0 100644
--- a/cmdutils.c
+++ b/cmdutils.c
@@ -1421,7 +1421,7 @@ static int compare_codec_desc(const void *a, const void *b)
const AVCodecDescriptor * const *da = a;
const AVCodecDescriptor * const *db = b;
- return (*da)->type != (*db)->type ? (*da)->type - (*db)->type :
+ return (*da)->type != (*db)->type ? FFDIFFSIGN((*da)->type, (*db)->type) :
strcmp((*da)->name, (*db)->name);
}
diff --git a/cmdutils_opencl.c b/cmdutils_opencl.c
index d9095b61be2..dd21344a056 100644
--- a/cmdutils_opencl.c
+++ b/cmdutils_opencl.c
@@ -206,7 +206,9 @@ end:
static int compare_ocl_device_desc(const void *a, const void *b)
{
- return ((const OpenCLDeviceBenchmark*)a)->runtime - ((const OpenCLDeviceBenchmark*)b)->runtime;
+ const OpenCLDeviceBenchmark* va = (const OpenCLDeviceBenchmark*)a;
+ const OpenCLDeviceBenchmark* vb = (const OpenCLDeviceBenchmark*)b;
+ return FFDIFFSIGN(va->runtime , vb->runtime);
}
int opt_opencl_bench(void *optctx, const char *opt, const char *arg)
diff --git a/ffmpeg.c b/ffmpeg.c
index f8b071a9cbf..d3b8c4d8010 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -2578,8 +2578,7 @@ static InputStream *get_input_stream(OutputStream *ost)
static int compare_int64(const void *a, const void *b)
{
- int64_t va = *(int64_t *)a, vb = *(int64_t *)b;
- return va < vb ? -1 : va > vb ? +1 : 0;
+ return FFDIFFSIGN(*(const int64_t *)a, *(const int64_t *)b);
}
static int init_output_stream(OutputStream *ost, char *error, int error_len)
diff --git a/libavfilter/f_sendcmd.c b/libavfilter/f_sendcmd.c
index 37aedc59f22..fb30220e7ca 100644
--- a/libavfilter/f_sendcmd.c
+++ b/libavfilter/f_sendcmd.c
@@ -364,11 +364,7 @@ static int cmp_intervals(const void *a, const void *b)
{
const Interval *i1 = a;
const Interval *i2 = b;
- int64_t ts_diff = i1->start_ts - i2->start_ts;
- int ret;
-
- ret = ts_diff > 0 ? 1 : ts_diff < 0 ? -1 : 0;
- return ret == 0 ? i1->index - i2->index : ret;
+ return 2 * FFDIFFSIGN(i1->start_ts, i2->start_ts) + FFDIFFSIGN(i1->index, i2->index);
}
static av_cold int init(AVFilterContext *ctx)
diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
index e32436d00c4..79fcc200e1b 100644
--- a/libavfilter/vf_deshake.c
+++ b/libavfilter/vf_deshake.c
@@ -94,8 +94,7 @@ AVFILTER_DEFINE_CLASS(deshake);
static int cmp(const void *a, const void *b)
{
- const double va = *(const double *)a, vb = *(const double *)b;
- return va < vb ? -1 : ( va > vb ? 1 : 0 );
+ return FFDIFFSIGN(*(const double *)a, *(const double *)b);
}
/**
diff --git a/libavfilter/vf_palettegen.c b/libavfilter/vf_palettegen.c
index df57c10982f..fccc5ca3fc0 100644
--- a/libavfilter/vf_palettegen.c
+++ b/libavfilter/vf_palettegen.c
@@ -130,7 +130,7 @@ static int cmp_color(const void *a, const void *b)
{
const struct range_box *box1 = a;
const struct range_box *box2 = b;
- return box1->color - box2->color;
+ return FFDIFFSIGN(box1->color , box2->color);
}
static av_always_inline int diff(const uint32_t a, const uint32_t b)
diff --git a/libavfilter/vf_removegrain.c b/libavfilter/vf_removegrain.c
index 3a28b15cdbe..898e50f101c 100644
--- a/libavfilter/vf_removegrain.c
+++ b/libavfilter/vf_removegrain.c
@@ -83,10 +83,9 @@ static int mode01(int c, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
static int cmp_int(const void *p1, const void *p2)
{
- int left = *(const int *)p1;
+ int left = *(const int *)p1;
int right = *(const int *)p2;
-
- return ((left > right) - (left < right));
+ return FFDIFFSIGN(left, right);
}
static int mode02(int c, int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)
diff --git a/libavformat/subtitles.c b/libavformat/subtitles.c
index 471d600c107..7c6cd5f3539 100644
--- a/libavformat/subtitles.c
+++ b/libavformat/subtitles.c
@@ -146,12 +146,9 @@ static int cmp_pkt_sub_ts_pos(const void *a, const void *b)
{
const AVPacket *s1 = a;
const AVPacket *s2 = b;
- if (s1->pts == s2->pts) {
- if (s1->pos == s2->pos)
- return 0;
- return s1->pos > s2->pos ? 1 : -1;
- }
- return s1->pts > s2->pts ? 1 : -1;
+ if (s1->pts == s2->pts)
+ return FFDIFFSIGN(s1->pos, s2->pos);
+ return FFDIFFSIGN(s1->pts , s2->pts);
}
static int cmp_pkt_sub_pos_ts(const void *a, const void *b)
--
GitLab