[FFmpeg-devel] [PATCH 1/2] avutil/internal: Add ff_elog()

[Ronald S. Bultje]

Hi,

On Thu, Feb 23, 2017 at 2:19 AM, wm4 <nfxjfg at googlemail.com> wrote:

> On Wed, 22 Feb 2017 19:16:46 +0100
> Michael Niedermayer <michael at niedermayer.cc> wrote:
>
> > This enables the extra error messages in case of DEBUG or high assrtion
> levels.
> > High assertion levels imply slow checks in inner loops so any extra
> error should
> > be insignificant.
> > Is it preferred to have a separate switch for ff_elog() so it doesnt
> depend on
> > DEBUG/assertion level ?
> >
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavutil/internal.h | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/libavutil/internal.h b/libavutil/internal.h
> > index 7780a9a791..208f8f474f 100644
> > --- a/libavutil/internal.h
> > +++ b/libavutil/internal.h
> > @@ -262,6 +262,12 @@ void avpriv_request_sample(void *avc,
> >  #   define ff_dlog(ctx, ...) do { if (0) av_log(ctx, AV_LOG_DEBUG,
> __VA_ARGS__); } while (0)
> >  #endif
> >
> > +#if defined(DEBUG) || ASSERT_LEVEL > 1
> > +#   define ff_elog(ctx, ...) av_log(ctx, AV_LOG_ERROR, __VA_ARGS__)
> > +#else
> > +#   define ff_elog(ctx, ...) do { if (0) av_log(ctx, AV_LOG_ERROR,
> __VA_ARGS__); } while (0)
> > +#endif
> > +
> >  // For debuging we use signed operations so overflows can be detected
> (by ubsan)
> >  // For production we use unsigned so there are no undefined operations
> >  #ifdef CHECKED
>
> Not really a fan of this. (And neither ff_dlog.) But I guess I don't
> mean to stop you.
>
> In my opinion, code that checks for overflow and fuzz issues should be
> as small and unintrusive as possible, or preferably not exist.


This ^^^ +100.

And to be clear, "code" means binary as well as source code size.

(Whether that applies to h264_ps specifically is a separate issue; as I
said on IRC a few days ago, it may well be that out-of-bounds values for
uv_qp_offset are common outside of fuzzing, e.g. some encoders creating
non-spec-compliant files. If that is really true, and we have examples of
such files, then I totally understand that you want a log message to aid
debugging, and maybe even a way to override them using -flags, as well as
an indication of this option/flag in the error message.)

Ronald