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

[Michael Niedermayer]

On Sat, Oct 31, 2015 at 10:47:54AM -0400, Ganesh Ajjanagadde wrote:
> This function can return ENOMEM that needs to be propagated.
> 
> Signed-off-by: Ganesh Ajjanagadde <gajjanagadde at gmail.com>
> ---
>  libavfilter/vf_pad.c    | 10 ++++++----
>  libavfilter/vf_rotate.c |  3 +--
>  libavfilter/vf_scale.c  |  5 +++--
>  libavfilter/vf_zscale.c |  5 +++--
>  4 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/libavfilter/vf_pad.c b/libavfilter/vf_pad.c
> index 63dc6a8..e10d41b 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)) < 0)
> +        goto eval_fail;
>      s->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
>      if ((ret = av_expr_parse_and_eval(&res, (expr = s->h_expr),
>                                        var_names, var_values,
> @@ -131,9 +132,10 @@ static int config_input(AVFilterLink *inlink)
>      s->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
>  
>      /* evaluate x and y */
> -    av_expr_parse_and_eval(&res, (expr = s->x_expr),
> +    if ((ret = av_expr_parse_and_eval(&res, (expr = s->x_expr),
>                             var_names, var_values,
> -                           NULL, NULL, NULL, NULL, NULL, 0, ctx);
> +                           NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> +        goto eval_fail;
>      s->x = var_values[VAR_X] = res;
>      if ((ret = av_expr_parse_and_eval(&res, (expr = s->y_expr),
>                                        var_names, var_values,
> diff --git a/libavfilter/vf_rotate.c b/libavfilter/vf_rotate.c
> index f12a103..d5e01e2 100644
> --- a/libavfilter/vf_rotate.c
> +++ b/libavfilter/vf_rotate.c
> @@ -235,8 +235,7 @@ static int config_props(AVFilterLink *outlink)
>  } while (0)
>  
>      /* evaluate width and height */
> -    av_expr_parse_and_eval(&res, expr = rot->outw_expr_str, var_names, rot->var_values,
> -                           func1_names, func1, NULL, NULL, rot, 0, ctx);
> +    SET_SIZE_EXPR(outw, "out_w");
>      rot->var_values[VAR_OUT_W] = rot->var_values[VAR_OW] = res;
>      rot->outw = res + 0.5;
>      SET_SIZE_EXPR(outh, "out_w");


> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 1032688..2cf14e0 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -265,9 +265,10 @@ static int config_props(AVFilterLink *outlink)
>      var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
>  
>      /* evaluate width and height */
> -    av_expr_parse_and_eval(&res, (expr = scale->w_expr),
> +    if ((ret = av_expr_parse_and_eval(&res, (expr = scale->w_expr),
>                             var_names, var_values,
> -                           NULL, NULL, NULL, NULL, NULL, 0, ctx);
> +                           NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> +        goto fail;
>      scale->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
>      if ((ret = av_expr_parse_and_eval(&res, (expr = scale->h_expr),
>                                        var_names, var_values,

the first evaluation can fail and such failure does not represent an
error for the calling code

this is needed to support referencing the output parameters,
for example
scale=iw/ih*oh*sar:240

here the first evalution of width fails but the 3rd succeeds as
the 2nd made "oh" available

please make sure the other hunks dont introduce similar issues
in other filters (theres similar such double eval in other filters)
also it would make sense to add comments to each such occurance of
double evaluation so noone else adds a check there by mistake

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20151101/92bdba87/attachment.sig>