[PATCH] Fix "git clone" for git:// protocol

From

Johannes Schindelin <johannes.schindelin@gmx.de>

Date

Feb 10, 2008, 03:06 UTC

Message-ID

<alpine.LSU.1.00.0802100302050.11591@racer.site>

In-Reply-To

<alpine.LNX.1.00.0802091205530.13593@iabervon.org>

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	
	On Sat, 9 Feb 2008, Daniel Barkalow wrote:

	> On Sat, 9 Feb 2008, Johannes Schindelin wrote:
	> 
	> > In the commit "Reduce the number of connects when fetching", 
	> > we checked the return value of git_connect() to see if the 
	> > connection was successful.
	> > 
	> > However, for the git:// protocol, there is no need to have 
	> > another process, so the return value is NULL.
	> > 
	> > The thing is: git_connect() does not return at all if it 
	> > fails, so we need not check the return value of git_connect().
	> 
	> Huh. Sure enough. Actually, there's a similar problem in 
	> transport.c, where it assumes that the return value of 
	> git_connect is non-zero, which makes it not reuse the connection 
	> (not that you can really tell). It might be good to roll in a 
	> fix for that. Or maybe git_connect should return a pointer to a 
	> static struct child_process if it doesn't need a subprocess, 
	> just to distinguish "we're doing it ourselves" from "it's not 
	> being done"? If not, maybe the variables that store the return 
	> from git_connect should be renamed to "subproc" or something 
	> that doesn't suggest they can't be NULL if you're actually 
	> connected.

	How about this?

 connect.c |   20 ++++++++++++--------
 1 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/connect.c b/connect.c
index 3aefd4a..700ceba 100644
--- a/connect.c
+++ b/connect.c
@@ -472,14 +472,18 @@ char *get_port(char *host)
 	return NULL;
 }
 
+static struct child_process no_fork;
+
 /*
- * This returns NULL if the transport protocol does not need fork(2), or a
- * struct child_process object if it does.  Once done, finish the connection
- * with finish_connect() with the value returned from this function
- * (it is safe to call finish_connect() with NULL to support the former
- * case).
+ * This returns a dummy child_process if the transport protocol does not
+ * need fork(2), or a struct child_process object if it does.  Once done,
+ * finish the connection with finish_connect() with the value returned from
+ * this function (it is safe to call finish_connect() with NULL to support
+ * the former case).
  *
- * If it returns, the connect is successful; it just dies on errors.
+ * If it returns, the connect is successful; it just dies on errors (this
+ * will hopefully be changed in a libification effort, to return NULL when
+ * the connection failed).
  */
 struct child_process *git_connect(int fd[2], const char *url_orig,
 				  const char *prog, int flags)
@@ -577,7 +581,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 		free(url);
 		if (free_path)
 			free(path);
-		return NULL;
+		return &no_fork;
 	}
 
 	conn = xcalloc(1, sizeof(*conn));
@@ -635,7 +639,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 int finish_connect(struct child_process *conn)
 {
 	int code;
-	if (!conn)
+	if (!conn || conn == &no_fork)
 		return 0;
 
 	code = finish_command(conn);

-- 
1.5.4.1264.g42770c