[PATCH] Fix memory leak in transport-helper

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Oct 27, 2009, 04:55 UTC
Message-ID: <alpine.LNX.2.00.0910270032170.14365@iabervon.org>
In-Reply-To: <20091015204543.GP10505@spearce.org>

On Thu, 15 Oct 2009, Shawn O. Pearce wrote:

Show 11 quoted lines

> The disconnect_helper function is not prepared to be called twice:
> 
> static int disconnect_helper(struct transport *transport)
> {
> 	struct helper_data *data = transport->data;
> 	if (data->helper) {
> 	...
> 	}
> 	free(data);
> 	return 0;
> }

Actually, my version just leaks transport->data; it looks like the "free(data);" line comes from your patch "remote-helpers: Support custom transport options". Here's a version (against origin/master) that neither leaks memory nor frees too much for disconnecting temporarily.

commit 8731d804c20828d20130e286f088613b5d33d57a
Author: Daniel Barkalow <barkalow@iabervon.org>
Date:   Tue Oct 27 00:42:16 2009 -0400

    Fix memory leak in helper method for disconnect.
    
    Since some cases may need to disconnect from the helper and reconnect,
    wrap the function that just disconnects in a function that also frees
    transport->data.
    
    Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>

diff --git a/transport-helper.c b/transport-helper.c
index f57e84c..479539d 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -67,6 +67,13 @@ static int disconnect_helper(struct transport *transport)
 	return 0;
 }
 
+static int close_helper(struct transport *transport)
+{
+	disconnect_helper(transport);
+	free(transport->data);
+	return 0;
+}
+
 static int fetch_with_fetch(struct transport *transport,
 			    int nr_heads, const struct ref **to_fetch)
 {
@@ -163,6 +170,6 @@ int transport_helper_init(struct transport *transport, const char *name)
 	transport->data = data;
 	transport->get_refs_list = get_refs_list;
 	transport->fetch = fetch;
-	transport->disconnect = disconnect_helper;
+	transport->disconnect = close_helper;
 	return 0;
 }