[libav-devel] [PATCH] assert: Use the asserts for their correct purpose

Anton Khirnov anton at khirnov.net
Mon Aug 31 11:34:58 CEST 2015


Quoting Luca Barbato (2015-08-31 05:30:29)
> assert(3) is a debugging aid. Do not leave it enabled
> on release builds.
> ---
> 
> At least some of those asserts should be converted to normal
> failure paths, I'm afraid. Help welcome.
> 
>  configure                       | 9 +++++++++
>  libavcodec/dvdsubenc.c          | 1 -
>  libavcodec/libschroedingerdec.c | 1 -
>  libavcodec/libvorbis.c          | 1 -
>  libavcodec/motion_est.c         | 1 -
>  libavcodec/mpegvideo_xvmc.c     | 1 -
>  libavcodec/qcelpdec.c           | 1 -
>  libavcodec/qdm2.c               | 1 -
>  libavcodec/ratecontrol.c        | 1 -
>  libavcodec/svq1dec.c            | 1 -
>  libavcodec/svq1enc.c            | 1 -
>  libavcodec/vc1.c                | 1 -
>  libavcodec/vdpau.c              | 1 -
>  libavcodec/vorbisenc.c          | 1 -
>  libavcodec/wma.c                | 1 -
>  libavcodec/wmadec.c             | 1 -
>  libavcodec/wmaenc.c             | 1 -
>  libavfilter/vf_yadif.c          | 1 -
>  libavformat/asfenc.c            | 1 -
>  libavformat/avidec.c            | 1 -
>  libavformat/flvenc.c            | 1 -
>  libavformat/mov.c               | 1 -
>  libavformat/movenc.c            | 1 -
>  libavformat/mpeg.c              | 1 -
>  libavformat/mpegenc.c           | 1 -
>  libavformat/mux.c               | 1 -
>  libavformat/nutdec.c            | 1 -
>  libavformat/utils.c             | 1 -
>  libavutil/avassert.h            | 4 ++--
>  29 files changed, 11 insertions(+), 29 deletions(-)
> 
> diff --git a/configure b/configure
> index c3c4f82..fd31099 100755
> --- a/configure
> +++ b/configure
> @@ -325,6 +325,7 @@ Developer options (useful when working on Libav itself):
>    --random-seed=VALUE      seed value for --enable/disable-random
>    --disable-valgrind-backtrace do not print a backtrace under Valgrind
>                             (only applies to --disable-optimizations builds)
> +  --enable-assert=LEVEL    enable asserts
> 
>  NOTE: Object files are built at the place where configure is launched.
>  EOF
> @@ -1650,6 +1651,7 @@ CMDLINE_SELECT="
>      $CONFIG_LIST
>      $HAVE_LIST_CMDLINE
>      $THREADS_LIST
> +    assert
>      asm
>      cross_compile
>      debug
> @@ -2513,6 +2515,9 @@ for opt do
>          --disable-devices)
>              disable $INDEV_LIST $OUTDEV_LIST
>          ;;
> +        --enable-assert=*)
> +            assertlevel="$optval"
> +        ;;
>          --enable-debug=*)
>              debuglevel="$optval"
>          ;;
> @@ -4521,6 +4526,7 @@ enabled vdpau && enabled xlib &&
>      enable vdpau_x11
> 
>  enabled debug && add_cflags -g"$debuglevel" && add_asflags -g"$debuglevel"
> +enabled assert || add_cflags -DNDEBUG
> 
>  # add some useful compiler flags if supported
>  check_cflags -Wdeclaration-after-statement
> @@ -5017,6 +5023,9 @@ cat > $TMPH <<EOF
>  #define SLIBSUF "$SLIBSUF"
>  EOF
> 
> +test -n "$assertlevel" &&
> +    echo "#define ASSERT_LEVEL $assertlevel" >>$TMPH
> +
>  test -n "$malloc_prefix" &&
>      echo "#define MALLOC_PREFIX $malloc_prefix" >>$TMPH
> 
> diff --git a/libavcodec/dvdsubenc.c b/libavcodec/dvdsubenc.c
> index f62e943e..2409d20 100644
> --- a/libavcodec/dvdsubenc.c
> +++ b/libavcodec/dvdsubenc.c
> @@ -21,7 +21,6 @@
>  #include "avcodec.h"
>  #include "bytestream.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  // ncnt is the nibble counter
> diff --git a/libavcodec/libschroedingerdec.c b/libavcodec/libschroedingerdec.c
> index d2594c6..08210cb 100644
> --- a/libavcodec/libschroedingerdec.c
> +++ b/libavcodec/libschroedingerdec.c
> @@ -37,7 +37,6 @@
>  #include "internal.h"
>  #include "libschroedinger.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
> 
> diff --git a/libavcodec/libvorbis.c b/libavcodec/libvorbis.c
> index 86c1ed6..a45da94 100644
> --- a/libavcodec/libvorbis.c
> +++ b/libavcodec/libvorbis.c
> @@ -35,7 +35,6 @@
>  #include "vorbis.h"
>  #include "vorbis_parser.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /* Number of samples the user should send in each call.
> diff --git a/libavcodec/motion_est.c b/libavcodec/motion_est.c
> index 657c28a..f135bb7 100644
> --- a/libavcodec/motion_est.c
> +++ b/libavcodec/motion_est.c
> @@ -38,7 +38,6 @@
>  #include "mpegutils.h"
>  #include "mpegvideo.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  #define P_LEFT P[1]
> diff --git a/libavcodec/mpegvideo_xvmc.c b/libavcodec/mpegvideo_xvmc.c
> index b7de79c..0b1ecf8 100644
> --- a/libavcodec/mpegvideo_xvmc.c
> +++ b/libavcodec/mpegvideo_xvmc.c
> @@ -26,7 +26,6 @@
>  #include "mpegutils.h"
>  #include "mpegvideo.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  #include "xvmc.h"
> diff --git a/libavcodec/qcelpdec.c b/libavcodec/qcelpdec.c
> index 2eeefb8..d515729 100644
> --- a/libavcodec/qcelpdec.c
> +++ b/libavcodec/qcelpdec.c
> @@ -40,7 +40,6 @@
>  #include "acelp_vectors.h"
>  #include "lsp.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  typedef enum {
> diff --git a/libavcodec/qdm2.c b/libavcodec/qdm2.c
> index 4736fac..f9a7be0 100644
> --- a/libavcodec/qdm2.c
> +++ b/libavcodec/qdm2.c
> @@ -47,7 +47,6 @@
>  #include "qdm2data.h"
>  #include "qdm2_tablegen.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
> 
> diff --git a/libavcodec/ratecontrol.c b/libavcodec/ratecontrol.c
> index c0eac6d..894e314 100644
> --- a/libavcodec/ratecontrol.c
> +++ b/libavcodec/ratecontrol.c
> @@ -35,7 +35,6 @@
>  #include "mpegvideo.h"
>  #include "libavutil/eval.h"
> 
> -#undef NDEBUG // Always check asserts, the speed effect is far too small to disable them.
>  #include <assert.h>
> 
>  #ifndef M_E
> diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> index 4f84395..73d2b76 100644
> --- a/libavcodec/svq1dec.c
> +++ b/libavcodec/svq1dec.c
> @@ -40,7 +40,6 @@
>  #include "mathops.h"
>  #include "svq1.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  static VLC svq1_block_type;
> diff --git a/libavcodec/svq1enc.c b/libavcodec/svq1enc.c
> index a869c24..a3963af 100644
> --- a/libavcodec/svq1enc.c
> +++ b/libavcodec/svq1enc.c
> @@ -37,7 +37,6 @@
>  #include "svq1enc.h"
>  #include "svq1enc_cb.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  static void svq1_write_header(SVQ1EncContext *s, int frame_type)
> diff --git a/libavcodec/vc1.c b/libavcodec/vc1.c
> index 0631439..04b115e 100644
> --- a/libavcodec/vc1.c
> +++ b/libavcodec/vc1.c
> @@ -37,7 +37,6 @@
>  #include "unary.h"
>  #include "simple_idct.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /***********************************************************************/
> diff --git a/libavcodec/vdpau.c b/libavcodec/vdpau.c
> index 8606624..05d00cb 100644
> --- a/libavcodec/vdpau.c
> +++ b/libavcodec/vdpau.c
> @@ -28,7 +28,6 @@
>  #include "h264.h"
>  #include "vc1.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  #include "vdpau.h"
> diff --git a/libavcodec/vorbisenc.c b/libavcodec/vorbisenc.c
> index 35bdd57..cd7e2cc 100644
> --- a/libavcodec/vorbisenc.c
> +++ b/libavcodec/vorbisenc.c
> @@ -36,7 +36,6 @@
>  #define BITSTREAM_WRITER_LE
>  #include "put_bits.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  typedef struct vorbis_enc_codebook {
> diff --git a/libavcodec/wma.c b/libavcodec/wma.c
> index f0aabfc..32c7d23 100644
> --- a/libavcodec/wma.c
> +++ b/libavcodec/wma.c
> @@ -29,7 +29,6 @@
>  #include "wma_freqs.h"
>  #include "wmadata.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /* XXX: use same run/length optimization as mpeg decoders */
> diff --git a/libavcodec/wmadec.c b/libavcodec/wmadec.c
> index b79dd2a..9cc5c11 100644
> --- a/libavcodec/wmadec.c
> +++ b/libavcodec/wmadec.c
> @@ -39,7 +39,6 @@
>  #include "internal.h"
>  #include "wma.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  #define EXPVLCBITS 8
> diff --git a/libavcodec/wmaenc.c b/libavcodec/wmaenc.c
> index c176daa..17bebed 100644
> --- a/libavcodec/wmaenc.c
> +++ b/libavcodec/wmaenc.c
> @@ -25,7 +25,6 @@
>  #include "internal.h"
>  #include "wma.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
> 
> diff --git a/libavfilter/vf_yadif.c b/libavfilter/vf_yadif.c
> index 53c567c..fe552f3 100644
> --- a/libavfilter/vf_yadif.c
> +++ b/libavfilter/vf_yadif.c
> @@ -29,7 +29,6 @@
>  #include "video.h"
>  #include "yadif.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  typedef struct ThreadData {
> diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
> index f6608d5..d8874ed 100644
> --- a/libavformat/asfenc.c
> +++ b/libavformat/asfenc.c
> @@ -27,7 +27,6 @@
>  #include "riff.h"
>  #include "asf.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
> 
> diff --git a/libavformat/avidec.c b/libavformat/avidec.c
> index 63afe2f..97e1979 100644
> --- a/libavformat/avidec.c
> +++ b/libavformat/avidec.c
> @@ -34,7 +34,6 @@
>  #include "isom.h"
>  #include "riff.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  typedef struct AVIStream {
> diff --git a/libavformat/flvenc.c b/libavformat/flvenc.c
> index 49c5235..7f7c6b7 100644
> --- a/libavformat/flvenc.c
> +++ b/libavformat/flvenc.c
> @@ -27,7 +27,6 @@
>  #include "internal.h"
>  #include "metadata.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  static const AVCodecTag flv_video_codec_ids[] = {
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 8bf1b90..e3d77a1 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -54,7 +54,6 @@
>  #include "qtpalette.h"
> 
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /* those functions parse an atom */
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index ee2f089..4737eab 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -44,7 +44,6 @@
>  #include "rtpenc.h"
>  #include "mov_chan.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  static const AVOption options[] = {
> diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
> index ffabe28..78013d0 100644
> --- a/libavformat/mpeg.c
> +++ b/libavformat/mpeg.c
> @@ -23,7 +23,6 @@
>  #include "internal.h"
>  #include "mpeg.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /*********************************************/
> diff --git a/libavformat/mpegenc.c b/libavformat/mpegenc.c
> index 33eaefd..ae86aa9 100644
> --- a/libavformat/mpegenc.c
> +++ b/libavformat/mpegenc.c
> @@ -35,7 +35,6 @@
> 
>  #define MAX_PAYLOAD_SIZE 4096
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  typedef struct PacketDesc {
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index 7959edf..fac5969 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -43,7 +43,6 @@
>  #include "network.h"
>  #endif
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  /**
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index bf1ab7b..18923d9 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -29,7 +29,6 @@
>  #include "nut.h"
>  #include "riff.h"
> 
> -#undef NDEBUG
>  #include <assert.h>
> 
>  #define NUT_MAX_STREAMS 256    /* arbitrary sanity check value */
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 5d4ec62..c7ab673 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -19,7 +19,6 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
> 
> -#undef NDEBUG
>  #include <assert.h>
>  #include <stdarg.h>
>  #include <stdint.h>
> diff --git a/libavutil/avassert.h b/libavutil/avassert.h
> index b223d26..2dbf143 100644
> --- a/libavutil/avassert.h
> +++ b/libavutil/avassert.h
> @@ -32,13 +32,13 @@
>  #include "log.h"
> 
>  /**
> - * assert() equivalent, that is always enabled.
> + * assert()-like macro with additional context
>   */
>  #define av_assert0(cond) do {                                           \
>      if (!(cond)) {                                                      \
>          av_log(NULL, AV_LOG_FATAL, "Assertion %s failed at %s:%d\n",    \
>                 AV_STRINGIFY(cond), __FILE__, __LINE__);                 \
> -        abort();                                                        \
> +        assert(0);                                                      \

I am against this. Assert means that the code in question relies on the
condition always being true. IMO it is far better and safer to abort()
immediately when it is false, rather than letting the code behave in
random unpredictable ways.

-- 
Anton Khirnov


More information about the libav-devel mailing list