connect: treat generic proxy processes like ssh processes

The git_connect function returns two ends of a pipe for talking with a remote, plus a struct child_process representing the other end of the pipe. If we have a direct socket connection, then this points to a special "no_fork" child process. The code path for doing git-over-pipes or git-over-ssh sets up this child process to point to the child git command or the ssh process. When we call finish_connect eventually, we check wait() on the command and report its return value. The code path for git://, on the other hand, always sets it to no_fork. In the case of a direct TCP connection, this makes sense; we have no child process. But in the case of a proxy command (configured by core.gitproxy), we do have a child process, but we throw away its pid, and therefore ignore its return code. Instead, let's keep that information in the proxy case, and respect its return code, which can help catch some errors (though depending on your proxy command, it will be errors reported by the proxy command itself, and not propagated from git commands. Still, it is probably better to propagate such errors than to ignore them). It also means that the child_process field can reliably be used to determine whether the returned descriptors are actually a full-duplex socket, which means we should be using shutdown() instead of a simple close. Signed-off-by: Jeff King <peff@peff.net> Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed May 16, 2011 at 02:46 UTC 5cbf8246d2e68470648d123e356665fca9ffca73
1 file changed +15 -13
connect.c
+15 -13
@@ -403,12 +403,12 @@ static int git_use_proxy(const char *host)
403 return (git_proxy_command && *git_proxy_command);
404 }
405
406 -static void git_proxy_connect(int fd[2], char *host)
406 +static struct child_process *git_proxy_connect(int fd[2], char *host)
407 {
408 const char *port = STR(DEFAULT_GIT_PORT);
409 char *colon, *end;
410 - const char *argv[4];
411 - struct child_process proxy;
410 + const char **argv;
411 + struct child_process *proxy;
412
413 if (host[0] == '[') {
414 end = strchr(host + 1, ']');
@@ -427,18 +427,20 @@ static void git_proxy_connect(int fd[2], char *host)
427 port = colon + 1;
428 }
429
430 + argv = xmalloc(sizeof(*argv) * 4);
431 argv[0] = git_proxy_command;
432 argv[1] = host;
433 argv[2] = port;
434 argv[3] = NULL;
434 - memset(&proxy, 0, sizeof(proxy));
435 - proxy.argv = argv;
436 - proxy.in = -1;
437 - proxy.out = -1;
438 - if (start_command(&proxy))
435 + proxy = xcalloc(1, sizeof(*proxy));
436 + proxy->argv = argv;
437 + proxy->in = -1;
438 + proxy->out = -1;
439 + if (start_command(proxy))
440 die("cannot start proxy %s", argv[0]);
440 - fd[0] = proxy.out; /* read from proxy stdout */
441 - fd[1] = proxy.in; /* write to proxy stdin */
441 + fd[0] = proxy->out; /* read from proxy stdout */
442 + fd[1] = proxy->in; /* write to proxy stdin */
443 + return proxy;
444 }
445
446 #define MAX_CMD_LEN 1024
@@ -479,7 +481,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
481 char *host, *path;
482 char *end;
483 int c;
482 - struct child_process *conn;
484 + struct child_process *conn = &no_fork;
485 enum protocol protocol = PROTO_LOCAL;
486 int free_path = 0;
487 char *port = NULL;
@@ -553,7 +555,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
555 */
556 char *target_host = xstrdup(host);
557 if (git_use_proxy(host))
556 - git_proxy_connect(fd, host);
558 + conn = git_proxy_connect(fd, host);
559 else
560 git_tcp_connect(fd, host, flags);
561 /*
@@ -571,7 +573,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
573 free(url);
574 if (free_path)
575 free(path);
574 - return &no_fork;
576 + return conn;
577 }
578
579 conn = xcalloc(1, sizeof(*conn));