[FFmpeg-devel] [PATCH 3/5] trace_headers: Fix memory leaks on syntax read failures

[James Almer]

On 10/4/2018 8:09 PM, Mark Thompson wrote:
> ---
>  libavcodec/trace_headers_bsf.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/trace_headers_bsf.c b/libavcodec/trace_headers_bsf.c
> index 94a3ef72a2..8322229d4c 100644
> --- a/libavcodec/trace_headers_bsf.c
> +++ b/libavcodec/trace_headers_bsf.c
> @@ -49,15 +49,11 @@ static int trace_headers_init(AVBSFContext *bsf)
>          av_log(bsf, AV_LOG_INFO, "Extradata\n");
>  
>          err = ff_cbs_read_extradata(ctx->cbc, &ps, bsf->par_in);
> -        if (err < 0) {
> -            av_log(bsf, AV_LOG_ERROR, "Failed to read extradata.\n");

Why remove this error message?

> -            return err;
> -        }
>  
>          ff_cbs_fragment_uninit(ctx->cbc, &ps);
>      }
>  
> -    return 0;
> +    return err;
>  }
>  
>  static void trace_headers_close(AVBSFContext *bsf)
> @@ -97,14 +93,12 @@ static int trace_headers(AVBSFContext *bsf, AVPacket *pkt)
>      av_log(bsf, AV_LOG_INFO, "Packet: %d bytes%s.\n", pkt->size, tmp);
>  
>      err = ff_cbs_read_packet(ctx->cbc, &au, pkt);
> -    if (err < 0) {
> -        av_packet_unref(pkt);
> -        return err;
> -    }
>  
>      ff_cbs_fragment_uninit(ctx->cbc, &au);

Maybe the ff_cbs_read* functions should clean whatever they were able to
allocate before the failure by calling this function internally, instead
of leaving it to the caller. It would be more consistent with what other
APIs we offer do.

>  
> -    return 0;
> +    if (err < 0)
> +        av_packet_unref(pkt);
> +    return err;
>  }
>  
>  const AVBitStreamFilter ff_trace_headers_bsf = {

LGTM in any case.