[FFmpeg-devel] [PATCH 6/7] avfilter/graphparser: Avoid check whose result is known in advance

[Nicolas George]

Andreas Rheinhardt (12020-08-23):
> The result of the last check in code like
> 
> if (p)
>     foo
> else
>     p = av_mallocz(sizeof(*p));
> 
> if (p->ptr)
>     bar
> else
>     bar2
> 
> is known in advance if the else branch of the first check was taken
> because av_mallocz() returns zeroed buffers. Therefore the above snippet
> can be simplified by moving the check and bar into the foo block.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/graphparser.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c
> index a52916a146..f55737f8c7 100644
> --- a/libavfilter/graphparser.c
> +++ b/libavfilter/graphparser.c
> @@ -266,20 +266,20 @@ static int link_filter_inouts(AVFilterContext *filt_ctx,
>          if (p) {
>              *curr_inputs = (*curr_inputs)->next;
>              p->next = NULL;
> +            if (p->filter_ctx) {
> +                ret = link_filter(p->filter_ctx, p->pad_idx, filt_ctx, pad, log_ctx);
> +                av_freep(&p->name);
> +                av_freep(&p);
> +                if (ret < 0)
> +                    return ret;
> +                continue;
> +            }
>          } else if (!(p = av_mallocz(sizeof(*p))))
>              return AVERROR(ENOMEM);
>  
> -        if (p->filter_ctx) {
> -            ret = link_filter(p->filter_ctx, p->pad_idx, filt_ctx, pad, log_ctx);
> -            av_freep(&p->name);
> -            av_freep(&p);
> -            if (ret < 0)
> -                return ret;
> -        } else {
> -            p->filter_ctx = filt_ctx;
> -            p->pad_idx = pad;
> -            append_inout(open_inputs, &p);
> -        }
> +        p->filter_ctx = filt_ctx;
> +        p->pad_idx = pad;
> +        append_inout(open_inputs, &p);
>      }
>  
>      if (*curr_inputs) {

I think I like the current version better. I had to read three times
before I noticed the "continue" that guarantees the last three lines
will not overwrite an existing filter. That means somebody could add a
line there, expecting it to be valid for both cases.

There is probably more simplification to be gained by reworking the
whole logic into a more modular structure, but that is a task for later.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200823/a70dce77/attachment.sig>