This is the first of what I expect to be several patches to convert
assertions of the
form "assert(a && b)" to "assert(a); assert(b)".
Here are some reasons why I think this would be an improvement. This
lengthy argument is not included in the patch attachment.
1. Assertion failures are often sporadic, and users who report them may
not be in a position to efficiently narrow them down further, so it
is important to get as much precision from each assertion failure as
possible.
2. It is a more efficient use of developers time when a bug report
arrives if the problem has been narrowed down that much more. A
new bug report may initially attract more interest, so, if the
problem has been narrowed down that much more, it may increase the
chance that developers may invest the time to try to resolve the
problem, and also reduce unnecessary traffic on the developer mailing
list about possible causes of the bug that separating the assertion
was able to rule out.
3. It's often more readable, sometimes eliminating parentheses or
changing multi-line conditions to separate single line conditions.
4. When using a debugger to step over an assertion failure in the
first part of the statement, the second part is still tested.
5. Providing separate likelihood hints to the compiler in the form
of separate assert statements does not require the compiler to
be quite as smart to recognize that it should optimize both branches,
although I do not know if that makes a difference for any compiler
commonly used to compile X (that is, I suspect that they are all
smart enough to realize is that "a && b" is likely true, then "a"
is likely true and "b" is likely true).
I have confirmed that the resulting tree built without any apparent
complaints about the assert statements and that "make fate" completed
with a zero exit code. I have not done any other tests though.
Thanks in advance for considering this patch.
Adam
From edb58a5ee8030ec66c04736a025d2a44e7322ba3 Mon Sep 17 00:00:00 2001
From: Adam Richter <adamricht...@gmail.com>
Date: Sun, 12 May 2019 03:41:49 -0700
Subject: [PATCH] libavformat: Separate assertions of the form
"av_assertN(a && b)" to "av_assertN(a); av_assertN(b)" for more precise
diagnostics.
Signed-off-by: Adam Richter <adamricht...@gmail.com>
---
libavformat/au.c | 3 ++-
libavformat/avienc.c | 3 ++-
libavformat/aviobuf.c | 3 ++-
libavformat/matroskaenc.c | 6 ++++--
libavformat/mov.c | 3 ++-
libavformat/rtmppkt.c | 3 ++-
libavformat/utils.c | 9 +++++----
7 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/libavformat/au.c b/libavformat/au.c
index cb48e67feb..76f23de56d 100644
--- a/libavformat/au.c
+++ b/libavformat/au.c
@@ -177,7 +177,8 @@ static int au_read_header(AVFormatContext *s)
bps = 2;
} else {
const uint8_t bpcss[] = {4, 0, 3, 5};
- av_assert0(id >= 23 && id < 23 + 4);
+ av_assert0(id >= 23);
+ av_assert0(id < 23 + 4);
ba = bpcss[id - 23];
bps = bpcss[id - 23];
}
diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index ac0f04c354..e96eb27e5e 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -797,7 +797,8 @@ static int avi_write_packet(AVFormatContext *s, AVPacket *pkt)
int pal_size = 1 << par->bits_per_coded_sample;
int pc_tag, i;
- av_assert0(par->bits_per_coded_sample >= 0 && par->bits_per_coded_sample <= 8);
+ av_assert0(par->bits_per_coded_sample >= 0);
+ av_assert0(par->bits_per_coded_sample <= 8);
if ((pb->seekable & AVIO_SEEKABLE_NORMAL) && avist->pal_offset) {
int64_t cur_offset = avio_tell(pb);
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 5a33f82950..e71846e0e8 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -195,7 +195,8 @@ static void flush_buffer(AVIOContext *s)
void avio_w8(AVIOContext *s, int b)
{
- av_assert2(b>=-128 && b<=255);
+ av_assert2(b >= -128);
+ av_assert2(b <= 255);
*s->buf_ptr++ = b;
if (s->buf_ptr >= s->buf_end)
flush_buffer(s);
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index cef504fa05..51b6d1b16f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -589,7 +589,8 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
tracks[j].has_cue = 0;
for (j = 0; j < cues->num_entries - i && entry[j].pts == pts; j++) {
int tracknum = entry[j].stream_idx;
- av_assert0(tracknum>=0 && tracknum<num_tracks);
+ av_assert0(tracknum >= 0);
+ av_assert0(tracknum < num_tracks);
if (tracks[tracknum].has_cue && s->streams[tracknum]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)
continue;
tracks[tracknum].has_cue = 1;
@@ -605,7 +606,8 @@ static int64_t mkv_write_cues(AVFormatContext *s, mkv_cues *cues, mkv_track *tra
tracks[j].has_cue = 0;
for (j = 0; j < cues->num_entries - i && entry[j].pts == pts; j++) {
int tracknum = entry[j].stream_idx;
- av_assert0(tracknum>=0 && tracknum<num_tracks);
+ av_assert0(tracknum >= 0);
+ av_assert0(tracknum < num_tracks);
if (tracks[tracknum].has_cue && s->streams[tracknum]->codecpar->codec_type != AVMEDIA_TYPE_SUBTITLE)
continue;
tracks[tracknum].has_cue = 1;
diff --git a/libavformat/mov.c b/libavformat/mov.c
index 78f692872b..c45ea416fc 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3267,7 +3267,8 @@ static void fix_index_entry_timestamps(AVStream* st, int end_index, int64_t end_
int64_t* frame_duration_buffer,
int frame_duration_buffer_size) {
int i = 0;
- av_assert0(end_index >= 0 && end_index <= st->nb_index_entries);
+ av_assert0(end_index >= 0);
+ av_assert0(end_index <= st->nb_index_entries);
for (i = 0; i < frame_duration_buffer_size; i++) {
end_ts -= frame_duration_buffer[frame_duration_buffer_size - 1 - i];
st->index_entries[end_index - 1 - i].timestamp = end_ts;
diff --git a/libavformat/rtmppkt.c b/libavformat/rtmppkt.c
index 1eeae17337..04b651d45e 100644
--- a/libavformat/rtmppkt.c
+++ b/libavformat/rtmppkt.c
@@ -501,7 +501,8 @@ int ff_amf_tag_size(const uint8_t *data, const uint8_t *data_end)
ret = amf_tag_skip(&gb);
if (ret < 0 || bytestream2_get_bytes_left(&gb) <= 0)
return -1;
- av_assert0(bytestream2_tell(&gb) >= 0 && bytestream2_tell(&gb) <= data_end - data);
+ av_assert0(bytestream2_tell(&gb) >= 0);
+ av_assert0(bytestream2_tell(&gb) <= data_end - data);
return bytestream2_tell(&gb);
}
diff --git a/libavformat/utils.c b/libavformat/utils.c
index a63d71b0f4..efd1b17588 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -163,10 +163,11 @@ void av_format_inject_global_side_data(AVFormatContext *s)
int ff_copy_whiteblacklists(AVFormatContext *dst, const AVFormatContext *src)
{
- av_assert0(!dst->codec_whitelist &&
- !dst->format_whitelist &&
- !dst->protocol_whitelist &&
- !dst->protocol_blacklist);
+ av_assert0(!dst->codec_whitelist);
+ av_assert0(!dst->format_whitelist);
+ av_assert0(!dst->protocol_whitelist);
+ av_assert0(!dst->protocol_blacklist);
+
dst-> codec_whitelist = av_strdup(src->codec_whitelist);
dst->format_whitelist = av_strdup(src->format_whitelist);
dst->protocol_whitelist = av_strdup(src->protocol_whitelist);
--
2.21.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
