Re: [PATCH 2/6] revert: Free memory after get_message call

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 11, 2011, 19:24 UTC
Message-ID: <20110811192403.GF2277@elie.gateway.2wire.net>
In-Reply-To: <1313088705-32222-3-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra wrote:

Show 5 quoted lines

> The format_todo function leaks memory because it forgets to call
> free_message after get_message.  Fix this.
>
> Suggested-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>

That's "Reported-by", I think. :)

Is this a big leak or a small one?  Is it one-time or in a loop?

Show 15 quoted lines

> ---
>  builtin/revert.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/builtin/revert.c b/builtin/revert.c
> index a548a14..1a4187a 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -688,6 +688,7 @@ static int format_todo(struct strbuf *buf, struct commit_list *todo_list,
>  			return error(_("Cannot get commit message for %s"), sha1_abbrev);
>  		strbuf_addf(buf, "%s %s %s\n", action_str, sha1_abbrev, msg.subject);
>  	}
> +	free_message(&msg);
>  	return 0;
>  }

I don't see how this could work. Since there an xmalloc() in each loop iteration, I would have expected the free() to be in the loop body, too.