[FFmpeg-devel] [PATCH 3/3] avutil/utils: remove unnecessary things

wm4 nfxjfg at googlemail.com
Tue Feb 10 13:27:04 CET 2015


The checks for the enums all were added in 2011 and 2012. They were
likely for checking that Libav changes do not change the FFmpeg ABI.
Given the amount of changes in the last 3 years, and the fact that
barely any other ABI specifics are tested, keeping these tests makes
not much sense anymore.

The assertion checking for "((size_t)-1) > 0" is useless. The condition
is guaranteed by the C standard for all usngined types, and size_t is
required to be an unsigned type. Why do we assume that compilers exist
which treat unsigned types as signed, and why do we assume that somebody
might try to compile FFmpeg with such a compiler, and why is this check
done at runtime? There is absolutely no value in checking for this: if
we assume that such a compiler exist, other compilers with _different_
bugs could randomly miscompile ffmpeg all over the place, even if this
check succeeds. (To make this worse, the commit the code comment points
to introduces a much more straight-forward assumption that size_t is
unsigned as demanded by the C standard: it replaces a "size<=0" check
with "!size".) Note that there are many other places in FFmpeg were
we (in parts rather sublty) rely on correct C semantics far beyond
correct behavior of unsigned types, even if the code is security
relevant.

The av_sat_dadd32() test was added in 2012 to check for broken ARM
binutils. Even if anyone argues that this is still relevant, it should
be done in configure instead. (And always should have.) The binutils
bug report (as cited in the original ffmpeg-devel thread discussing
this patch) was marked as fixed in 2009.

The llrint() was added in 2013 for NetBSD. I don't see what printing a
message changes. (Apparently nothing; from what I can see NetBSD didn't
touch its llrint implementation since 2004.) If it's critical, just make
configure refuse to run if NetBSD is detected, or print a compile-time
warning, or do not use llrint.
---
 libavutil/utils.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/libavutil/utils.c b/libavutil/utils.c
index 0b765ed..e79e8bc 100644
--- a/libavutil/utils.c
+++ b/libavutil/utils.c
@@ -32,29 +32,6 @@ const char av_util_ffversion[] = "FFmpeg version " FFMPEG_VERSION;
 
 unsigned avutil_version(void)
 {
-    static int checks_done;
-    if (checks_done)
-        return LIBAVUTIL_VERSION_INT;
-
-    av_assert0(AV_PIX_FMT_VDA_VLD == 81); //check if the pix fmt enum has not had anything inserted or removed by mistake
-    av_assert0(AV_SAMPLE_FMT_DBLP == 9);
-    av_assert0(AVMEDIA_TYPE_ATTACHMENT == 4);
-    av_assert0(AV_PICTURE_TYPE_BI == 7);
-    av_assert0(LIBAVUTIL_VERSION_MICRO >= 100);
-    av_assert0(HAVE_MMX2 == HAVE_MMXEXT);
-
-    av_assert0(((size_t)-1) > 0); // C guarantees this but if false on a platform we care about revert at least b284e1ffe343d6697fb950d1ee517bafda8a9844
-
-    if (av_sat_dadd32(1, 2) != 5) {
-        av_log(NULL, AV_LOG_FATAL, "Libavutil has been build with a broken binutils, please upgrade binutils and rebuild\n");
-        abort();
-    }
-
-    if (llrint(1LL<<60) != 1LL<<60) {
-        av_log(NULL, AV_LOG_ERROR, "Libavutil has been linked to a broken llrint()\n");
-    }
-
-    checks_done = 1;
     return LIBAVUTIL_VERSION_INT;
 }
 
-- 
2.1.4



More information about the ffmpeg-devel mailing list