Re: [FFmpeg-devel] [PATCH v2 1/3] avformat/sapenc: fix leak in sap_write_header()

From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v2 1/3] avformat/sapenc: fix leak in sap_write_header()
Date: Sat, 12 Jul 2025 20:33:03 +0200
Message-ID: <20250712183303.GC29660@pb2> (raw)
In-Reply-To: <20250710022047.1847641-2-502024330056@smail.nju.edu.cn>

[-- Attachment #1.1: Type: text/plain, Size: 1929 bytes --]

On Thu, Jul 10, 2025 at 10:20:45AM +0800, Lidong Yan wrote:
> In sap_write_header(), ff_format_set_url() assign new allocated new_url
> to contexts[i]->url but forgot to free it later. Add for loop to free
> contexts[i]->url before av_free(context).
> 
> To prevent from writing free-for-loop in every return point, replace
> `return 0` with `ret = 0` so normal execution can fall through fail
> code.
> 
> Signed-off-by: Lidong Yan <502024330056@smail.nju.edu.cn>
> ---
>  libavformat/sapenc.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/libavformat/sapenc.c b/libavformat/sapenc.c
> index 87a834a8d8..0567a754e2 100644
> --- a/libavformat/sapenc.c
> +++ b/libavformat/sapenc.c
> @@ -233,7 +233,6 @@ static int sap_write_header(AVFormatContext *s)
>          ret = AVERROR_INVALIDDATA;
>          goto fail;
>      }
> -    av_freep(&contexts);
>      av_log(s, AV_LOG_VERBOSE, "SDP:\n%s\n", &sap->ann[pos]);
>      pos += strlen(&sap->ann[pos]);
>      sap->ann_size = pos;
> @@ -244,11 +243,17 @@ static int sap_write_header(AVFormatContext *s)
>          goto fail;
>      }
>  
> -    return 0;
> +    ret = 0;
>  
>  fail:
> -    av_free(contexts);
> -    sap_write_close(s);
> +    if (contexts) {
> +        for (i = 0; i < s->nb_streams; i++)
> +            if (contexts[i])
> +                av_free(contexts[i]->url);
> +        av_free(contexts);
> +    }

contexts is an array of AVFormatContext, this does not look right
also freeing the url of the AVFormatContexts that have been stored
in priv_data a few lines earlier looks wrong

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".