In ba227857(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 was NULL. Now, it makes sense to assume the rule that git_connect() will return NULL if it fails (at the moment, it die()s if it fails), so return a dummy child process. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> --- 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 - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html