Re: [REGRESSION, BISECTED] `git checkout <branch>` started to be memory hog

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 22, 2011, 17:33 UTC
Message-ID: <7vipquz0d0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20110722170001.GB20700@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 20 quoted lines

> From my quick look, I came up with the fix below. It removes the leak
> and doesn't trigger any memory errors according to valgrind. So it
> _must_ be right. :)
>
> -Peff
>
> ---
> diff --git a/streaming.c b/streaming.c
> index 565f000..f3acc5d 100644
> --- a/streaming.c
> +++ b/streaming.c
> @@ -93,7 +93,9 @@ struct git_istream {
>  
>  int close_istream(struct git_istream *st)
>  {
> -	return st->vtbl->close(st);
> +	int r = st->vtbl->close(st);
> +	free(st);
> +	return r;
>  }

I do not think of a reason any future callers would want to close the stream first and then inspect some attribute of the stream afterwards (if this were an output stream, there might be things like output stats that may want such an interface, but even in such a case, the caller should say "flush" first to drain whatever is pending, grab stats and then close), so freeing the resource at close time seems sensible to me.

Both the external caller in entry.c and the internal caller (a filtered istream reads from its underlying istream, and when it is getting closed, closes the underlying istream) were leaking the istream exactly the same way, so this fix should plug both of them.

Thanks.