[FFmpeg-devel] [PATCH 5/6] avfilter/formats: Simplify cleanup for ff_merge_* functions

[Nicolas George]

Andreas Rheinhardt (12020-08-08):
> Now that the output's refs-array is only allocated once, it is NULL in
> any error case and therefore needn't be freed at all. Furthermore, it is
> unnecessary to av_freep(&ptr) when ptr == NULL.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>  libavfilter/formats.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/libavfilter/formats.c b/libavfilter/formats.c
> index ae44006dfe..e452fd0408 100644
> --- a/libavfilter/formats.c
> +++ b/libavfilter/formats.c
> @@ -141,10 +141,9 @@ AVFilterFormats *ff_merge_formats(AVFilterFormats *a, AVFilterFormats *b,
>      return ret;
>  fail:
>      if (ret) {

> -        av_freep(&ret->refs);

It seems fragile. We could easily unmake the change that guarantees that
refs is NULL, or use fail for another exit later. Maybe add an
av_assert1()?

>          av_freep(&ret->formats);

> +        av_freep(&ret);
>      }
> -    av_freep(&ret);

This one ok, of course.

>      return NULL;
>  }
>  
> @@ -168,10 +167,9 @@ AVFilterFormats *ff_merge_samplerates(AVFilterFormats *a,
>      return ret;
>  fail:
>      if (ret) {
> -        av_freep(&ret->refs);
>          av_freep(&ret->formats);
> +        av_freep(&ret);
>      }
> -    av_freep(&ret);
>      return NULL;
>  }
>  
> @@ -261,10 +259,9 @@ AVFilterChannelLayouts *ff_merge_channel_layouts(AVFilterChannelLayouts *a,
>  
>  fail:
>      if (ret) {
> -        av_freep(&ret->refs);
>          av_freep(&ret->channel_layouts);
> +        av_freep(&ret);
>      }
> -    av_freep(&ret);
>      return NULL;
>  }

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/20200808/eaf9503c/attachment.sig>