[FFmpeg-devel] [PATCH 1/9] avutil: add FF_RETURN_ON_OVERFLOW

[James Almer]

On 1/6/2017 4:46 PM, Andreas Cadhalpun wrote:
> Suggested-by: Rodger Combs <rodger.combs at gmail.com>
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
> 
> Changed the name as suggested by wm4 and the return value as suggested
> by Muhammad Faiz.
> There are also two new overflow checks at the end of the series.

This is a good chance to introduce the gcc overflow check builtins.
See https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html, they
use hardware instructions when possible, like x86's Jump on Overflow.

The idea would be to use __builtin_mul_overflow_p(). For example (untested):

#if AV_GCC_VERSION_AT_LEAST(5,1)
#define av_builtin_mul_overflow_p(a, b) __builtin_mul_overflow_p(a, b, (int) 0)
#else
#define av_builtin_mul_overflow_p(a, b) ((a) > INT_MAX / (b)))
#endif

It can also be used all across the codebase, not just for these checks.

> 
> ---
>  libavutil/common.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavutil/common.h b/libavutil/common.h
> index 8142b31fdb..6d795a353a 100644
> --- a/libavutil/common.h
> +++ b/libavutil/common.h
> @@ -99,6 +99,8 @@
>  #define FFSWAP(type,a,b) do{type SWAP_tmp= b; b= a; a= SWAP_tmp;}while(0)
>  #define FF_ARRAY_ELEMS(a) (sizeof(a) / sizeof((a)[0]))
>  
> +#define FF_RETURN_ON_OVERFLOW(ctx, x) if (x) {av_log(ctx, AV_LOG_ERROR, "Overflow check failed: " #x"\n"); return AVERROR(ERANGE);}

Printing an error unconditionally seems like log bloat. We do all kinds of
sanity checks on demuxers and simply return INVALIDDATA without printing
anything if they fail.
Maybe we should do the same here and let the demuxer choose to print an
error or not.

> +
>  /* misc math functions */
>  
>  #ifdef HAVE_AV_CONFIG_H
>