Re: [PATCH] Fix memory leak in function handle_content_type
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 16, 2025, 01:01 UTC
- Message-ID
- <xmqqldps4i00.fsf@gitster.g>
- In-Reply-To
- <8C3C7F5F-9442-4C63-9280-D46A683C9F4D@gmail.com>
Lidong Yan <yldhome2d2@gmail.com> writes:
> Subject: Re: [PATCH] Fix memory leak in function handle_content_type
The subject should probably be something like
Subject: [PATCH] mailinfo.c: plug memory leak in handle_content_type()
Show 24 quoted lines
> 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);
> —Yup, that one looks good enough.
If we wanted to clean up this code path even further, I think the first clean-up to do is to reconsider the use of "struct strbuf *" (instead of "const char *") in *(mi->content_top). strbuf is a fine and less error-prone mechanism to use while you have to manipulate character strings (like parsing from input line to formulate the boundary string), but once this function computed what was asked by the caller, the computed result (like the boundary string) almost always do not need to be editable. But such a code improvement is totally outside the topic of this patch.