[FFmpeg-devel] [PATCHv2 1/4] avfilter/all: check and propagate the return code of av_expr_parse_and_eval

[Ganesh Ajjanagadde]

On Sun, Nov 1, 2015 at 3:45 PM, Nicolas George <george at nsup.org> wrote:
> Le primidi 11 brumaire, an CCXXIV, Ganesh Ajjanagadde a écrit :
>> This function can return ENOMEM that needs to be propagated.
>>
>> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
>> ---
>>  libavfilter/vf_pad.c    | 12 +++++++-----
>>  libavfilter/vf_rotate.c |  5 +++--
>>  libavfilter/vf_scale.c  |  5 +++--
>>  libavfilter/vf_zscale.c |  5 +++--
>>  4 files changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
>> index 63dc6a8..a40f5fa 100644
>> --- a/libavfilter/vf_pad.c
>> +++ b/libavfilter/vf_pad.c
>> @@ -114,9 +114,10 @@ static int config_input(AVFilterLink *inlink)
>>      var_values[VAR_VSUB]  = 1 << s->draw.vsub_max;
>>
>>      /* evaluate width and height */
>> -    av_expr_parse_and_eval(&res, (expr = s->w_expr),
>> +    if ((ret = av_expr_parse_and_eval(&res, (expr = s->w_expr),
>>                             var_names, var_values,
>> -                           NULL, NULL, NULL, NULL, NULL, 0, ctx);
>> +                           NULL, NULL, NULL, NULL, NULL, 0, ctx)) == AVERROR(ENOMEM))
>> +        goto eval_fail;
>
> I am quite unhappy about this, it is cluttering the code for no good reason
> and makes the test fragile.

The fragility is something I don't like either, suggestions?

As for the cluttering, there are lots of if (!...) return
AVERROR(ENOMEM) all over the place. I assumed that people want such
things checked in essentially all circumstances since OOM can hit
anywhere. If that is not the case, please help by giving an
explanation of when they do/do not matter so that I can focus better
on the more  essential ones.

>
> Regards,
>
> --
>   Nicolas George
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>