Re: [PATCH] Fix memory leak in function handle_content_type

[Junio C Hamano <gitster@pobox.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()

> 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.