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

[Paul B Mahol]

On 1/7/17, Michael Niedermayer <michael at niedermayer.cc> wrote:
> On Fri, Jan 06, 2017 at 09:11:10PM -0300, James Almer wrote:
>> 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.
>
> error messages help debuging and thus maintaining code.
>

Not really. Expecially in this case.