From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 23 Mar 2015 17:27:13 GMT
Subject: Re: [PATCH] builtin/apply.c: fix a memleak (Fixup, squashable)
Message-ID: <xmqqwq27aaku.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1427130811-21222-1-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>
>> By the way, aren't the following also leaking here?
>>
>>  - the strbuf newlines that starts out as "size"
>>  - line[] arrays of preimage and postimage
>>
>
> yes :(
>
> I think a viable replacement may be below.

Yeah, that looks a lot more sensible and straight-forward.

Thanks.

> That way the returned value of the function is not -1, but 1.
> We're using the return value only as a boolean, so it doesn't
> really matter.
>
>  builtin/apply.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>  
> diff --git a/builtin/apply.c b/builtin/apply.c
> index e152c4d..0769b09 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -2776,8 +2776,8 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
>  		default:
>  			if (apply_verbosely)
>  				error(_("invalid start of line: '%c'"), first);
> -			free(oldlines);
> -			return -1;
> +			applied_pos = -1;
> +			goto out;
>  		}
>  		if (added_blank_line) {
>  			if (!new_blank_lines_at_end)
> @@ -2916,6 +2916,7 @@ static int apply_one_fragment(struct image *img, struct fragment *frag,
>  			      (int)(old - oldlines), oldlines);
>  	}
>  
> +out:
>  	free(oldlines);
>  	strbuf_release(&newlines);
>  	free(preimage.line_allocated);