Re: [PATCH] Fix memory leak in function handle_content_type

From: Lidong Yan <yldhome2d2@gmail.com>
Date: Jun 15, 2025, 12:55 UTC
Message-ID: <8C3C7F5F-9442-4C63-9280-D46A683C9F4D@gmail.com>
In-Reply-To: <SA1PR22MB399911638F342E1AA20F014AE477A@SA1PR22MB3999.namprd22.prod.outlook.com>

Jinyao Guo <guo846@purdue.edu> writes：

Show 36 quoted lines

> 
> Sure. I'll choose one account and use it consistently.
> 
> Here is the edited patch:
> 
> From 04b286cb2e736c3a53287b6ddf406e704f19fb2e Mon Sep 17 00:00:00 2001
> From: jinyaoguo <guo846@purdue.edu>
> Date: Thu, 12 Jun 2025 18:48:24 -0400
> Subject: [PATCH] Fix memory leak in function handle_content_type
> 
> The function handle_content_type allocates memory for boundary
> using xmalloc(sizeof(struct strbuf)). If (++mi->content_top >=
> &mi->content[MAX_BOUNDARIES]) is true, the function returns
> without freeing boundary.
> 
> Signed-off-by: jinyaoguo <guo846@purdue.edu>
> ---
> mailinfo.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/mailinfo.c b/mailinfo.c
> index ee4597da6b..e0ea358311 100644
> --- a/mailinfo.c
> +++ b/mailinfo.c
> @@ -266,6 +266,9 @@ static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
> error("Too many boundaries to handle");
> mi->input_error = -1;
> mi->content_top = &mi->content[MAX_BOUNDARIES] - 1;
> + strbuf_release(boundary);
> + free(boundary);
> + boundary = NULL;
> return;
> }
> *(mi->content_top) = boundary;
> -- 
> 2.34.1

May be using goto here would be better. Like:

---

diff --git a/mailinfo.c b/mailinfo.c
index ee4597da6b..83358b7517 100644
--- a/mailinfo.c
+++ b/mailinfo.c
@@ -266,13 +266,14 @@ static void handle_content_type(struct mailinfo *mi, struct strbuf *line)
                        error("Too many boundaries to handle");
                        mi->input_error = -1;
                        mi->content_top = &mi->content[MAX_BOUNDARIES] - 1;
-                       return;
+                       goto out;
                }
                *(mi->content_top) = boundary;
                boundary = NULL;
        }
        slurp_attr(line->buf, "charset=", &mi->charset);
 
+out:
        if (boundary) {
                strbuf_release(boundary);
                free(boundary);
—

Lidong