Skip to content

Commit 92e483f

Browse files
author
Ganesh Ajjanagadde
committed
all: use FFDIFFSIGN to resolve possible undefined behavior in comparators
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 <[email protected]> Reviewed-by: Clément Bœsch <[email protected]> Signed-off-by: Ganesh Ajjanagadde <[email protected]>
1 parent 265f83f commit 92e483f

File tree

8 files changed

+13
-21
lines changed

8 files changed

+13
-21
lines changed

cmdutils.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -1421,7 +1421,7 @@ static int compare_codec_desc(const void *a, const void *b)
14211421
const AVCodecDescriptor * const *da = a;
14221422
const AVCodecDescriptor * const *db = b;
14231423

1424-
return (*da)->type != (*db)->type ? (*da)->type - (*db)->type :
1424+
return (*da)->type != (*db)->type ? FFDIFFSIGN((*da)->type, (*db)->type) :
14251425
strcmp((*da)->name, (*db)->name);
14261426
}
14271427

cmdutils_opencl.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,9 @@ static int64_t run_opencl_bench(AVOpenCLExternalEnv *ext_opencl_env)
206206

207207
static int compare_ocl_device_desc(const void *a, const void *b)
208208
{
209-
return ((const OpenCLDeviceBenchmark*)a)->runtime - ((const OpenCLDeviceBenchmark*)b)->runtime;
209+
const OpenCLDeviceBenchmark* va = (const OpenCLDeviceBenchmark*)a;
210+
const OpenCLDeviceBenchmark* vb = (const OpenCLDeviceBenchmark*)b;
211+
return FFDIFFSIGN(va->runtime , vb->runtime);
210212
}
211213

212214
int opt_opencl_bench(void *optctx, const char *opt, const char *arg)

ffmpeg.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -2578,8 +2578,7 @@ static InputStream *get_input_stream(OutputStream *ost)
25782578

25792579
static int compare_int64(const void *a, const void *b)
25802580
{
2581-
int64_t va = *(int64_t *)a, vb = *(int64_t *)b;
2582-
return va < vb ? -1 : va > vb ? +1 : 0;
2581+
return FFDIFFSIGN(*(const int64_t *)a, *(const int64_t *)b);
25832582
}
25842583

25852584
static int init_output_stream(OutputStream *ost, char *error, int error_len)

libavfilter/f_sendcmd.c

+1-5
Original file line numberDiff line numberDiff line change
@@ -364,11 +364,7 @@ static int cmp_intervals(const void *a, const void *b)
364364
{
365365
const Interval *i1 = a;
366366
const Interval *i2 = b;
367-
int64_t ts_diff = i1->start_ts - i2->start_ts;
368-
int ret;
369-
370-
ret = ts_diff > 0 ? 1 : ts_diff < 0 ? -1 : 0;
371-
return ret == 0 ? i1->index - i2->index : ret;
367+
return 2 * FFDIFFSIGN(i1->start_ts, i2->start_ts) + FFDIFFSIGN(i1->index, i2->index);
372368
}
373369

374370
static av_cold int init(AVFilterContext *ctx)

libavfilter/vf_deshake.c

+1-2
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ AVFILTER_DEFINE_CLASS(deshake);
9494

9595
static int cmp(const void *a, const void *b)
9696
{
97-
const double va = *(const double *)a, vb = *(const double *)b;
98-
return va < vb ? -1 : ( va > vb ? 1 : 0 );
97+
return FFDIFFSIGN(*(const double *)a, *(const double *)b);
9998
}
10099

101100
/**

libavfilter/vf_palettegen.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ static int cmp_color(const void *a, const void *b)
130130
{
131131
const struct range_box *box1 = a;
132132
const struct range_box *box2 = b;
133-
return box1->color - box2->color;
133+
return FFDIFFSIGN(box1->color , box2->color);
134134
}
135135

136136
static av_always_inline int diff(const uint32_t a, const uint32_t b)

libavfilter/vf_removegrain.c

+2-3
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,9 @@ static int mode01(int c, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
8383

8484
static int cmp_int(const void *p1, const void *p2)
8585
{
86-
int left = *(const int *)p1;
86+
int left = *(const int *)p1;
8787
int right = *(const int *)p2;
88-
89-
return ((left > right) - (left < right));
88+
return FFDIFFSIGN(left, right);
9089
}
9190

9291
static int mode02(int c, int a1, int a2, int a3, int a4, int a5, int a6, int a7, int a8)

libavformat/subtitles.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,9 @@ static int cmp_pkt_sub_ts_pos(const void *a, const void *b)
146146
{
147147
const AVPacket *s1 = a;
148148
const AVPacket *s2 = b;
149-
if (s1->pts == s2->pts) {
150-
if (s1->pos == s2->pos)
151-
return 0;
152-
return s1->pos > s2->pos ? 1 : -1;
153-
}
154-
return s1->pts > s2->pts ? 1 : -1;
149+
if (s1->pts == s2->pts)
150+
return FFDIFFSIGN(s1->pos, s2->pos);
151+
return FFDIFFSIGN(s1->pts , s2->pts);
155152
}
156153

157154
static int cmp_pkt_sub_pos_ts(const void *a, const void *b)

0 commit comments

Comments
 (0)