[FFmpeg-devel] [PATCH 1/4] avformat/tests/fifo_muxer: Fix leak of AVPacket on error

[James Almer]

On 3/18/2021 9:52 PM, Andreas Rheinhardt wrote:
> Also factor allocating and freeing the packet out.
> Fixes Coverity issues #1473722 and #1473723; it is a regression
> since 4b386b2059806ca7ee7f991d2c8b735410693e8c.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
> ---
>   libavformat/tests/fifo_muxer.c | 37 ++++++++++++++--------------------
>   1 file changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/libavformat/tests/fifo_muxer.c b/libavformat/tests/fifo_muxer.c
> index 3458c3eefd..e8970259c4 100644
> --- a/libavformat/tests/fifo_muxer.c
> +++ b/libavformat/tests/fifo_muxer.c
> @@ -55,7 +55,7 @@ static int prepare_packet(AVPacket *pkt, const FailingMuxerPacketData *pkt_data,
>       return ret;
>   }
>   
> -static int initialize_fifo_tst_muxer_chain(AVFormatContext **oc)
> +static int initialize_fifo_tst_muxer_chain(AVFormatContext **oc, AVPacket **pkt)
>   {
>       int ret = 0;
>       AVStream *s;
> @@ -71,22 +71,20 @@ static int initialize_fifo_tst_muxer_chain(AVFormatContext **oc)
>       if (!s) {
>           fprintf(stderr, "Failed to create stream: %s\n",
>                   av_err2str(ret));
> -        ret = AVERROR(ENOMEM);
> +        return AVERROR(ENOMEM);
>       }
>   
> -    return ret;
> +    *pkt = av_packet_alloc();
> +    if (!*pkt)
> +        return AVERROR(ENOMEM);
> +
> +    return 0;
>   }
>   
>   static int fifo_basic_test(AVFormatContext *oc, AVDictionary **opts,
> -                             const FailingMuxerPacketData *pkt_data)
> +                           AVPacket *pkt, const FailingMuxerPacketData *pkt_data)
>   {
>       int ret = 0, i;
> -    AVPacket *pkt;
> -
> -    pkt = av_packet_alloc();
> -    if (!pkt)
> -        return AVERROR(ENOMEM);
> -
>   
>       ret = avformat_write_header(oc, opts);
>       if (ret) {
> @@ -107,11 +105,9 @@ static int fifo_basic_test(AVFormatContext *oc, AVDictionary **opts,
>           if (ret < 0) {
>               fprintf(stderr, "Unexpected write_frame error: %s\n",
>                       av_err2str(ret));
> -            av_packet_free(&pkt);
>               goto write_trailer_and_fail;
>           }
>       }
> -    av_packet_free(&pkt);
>   
>       ret = av_write_frame(oc, NULL);
>       if (ret < 0) {
> @@ -135,15 +131,10 @@ fail:
>   }
>   
>   static int fifo_overflow_drop_test(AVFormatContext *oc, AVDictionary **opts,
> -                                   const FailingMuxerPacketData *data)
> +                                   AVPacket *pkt, const FailingMuxerPacketData *data)
>   {
>       int ret = 0, i;
>       int64_t write_pkt_start, write_pkt_end, duration;
> -    AVPacket *pkt;
> -
> -    pkt = av_packet_alloc();
> -    if (!pkt)
> -        return AVERROR(ENOMEM);
>   
>       ret = avformat_write_header(oc, opts);
>       if (ret) {
> @@ -166,7 +157,6 @@ static int fifo_overflow_drop_test(AVFormatContext *oc, AVDictionary **opts,
>               break;
>           }
>       }
> -    av_packet_free(&pkt);
>   
>       write_pkt_end = av_gettime_relative();
>       duration = write_pkt_end - write_pkt_start;
> @@ -193,7 +183,8 @@ fail:
>   }
>   
>   typedef struct TestCase {
> -    int (*test_func)(AVFormatContext *, AVDictionary **,const FailingMuxerPacketData *pkt_data);
> +    int (*test_func)(AVFormatContext *, AVDictionary **,
> +                     AVPacket *, const FailingMuxerPacketData *pkt_data);
>       const char *test_name;
>       const char *options;
>   
> @@ -211,10 +202,11 @@ static int run_test(const TestCase *test)
>   {
>       AVDictionary *opts = NULL;
>       AVFormatContext *oc = NULL;
> +    AVPacket *pkt = NULL;
>       char buffer[BUFFER_SIZE];
>       int ret, ret1;
>   
> -    ret = initialize_fifo_tst_muxer_chain(&oc);
> +    ret = initialize_fifo_tst_muxer_chain(&oc, &pkt);
>       if (ret < 0) {
>           fprintf(stderr, "Muxer initialization failed: %s\n", av_err2str(ret));
>           goto end;
> @@ -240,11 +232,12 @@ static int run_test(const TestCase *test)
>           goto end;
>       }
>   
> -    ret = test->test_func(oc, &opts, &test->pkt_data);
> +    ret = test->test_func(oc, &opts, pkt, &test->pkt_data);
>   
>   end:
>       printf("%s: %s\n", test->test_name, ret < 0 ? "fail" : "ok");
>       avformat_free_context(oc);
> +    av_packet_free(&pkt);
>       av_dict_free(&opts);
>       return ret;
>   }

Should be ok if tested.