git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v7 10/10] git_connect(): Use common return point

From
Torsten Bögershausen <tboegi@web.de>
Date
Nov 28, 2013, 19:50 UTC
Message-ID
<201311282050.16371.tboegi@web.de>

Use only one return point from git_connect(), doing the free(): return conn; only at one place in the code.

There may be a little confusion what the variable "host" is for. At some places it is only the host part, at other places it may include the port number, so change host into hostandport here.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 connect.c | 108 +++++++++++++++++++++++++++++---------------------------------
 1 file changed, 50 insertions(+), 58 deletions(-)
diff --git a/connect.c b/connect.c
index 04093c4..8a013a7 100644
--- a/connect.c
+++ b/connect.c
@@ -656,7 +656,7 @@ static struct child_process no_fork;
 struct child_process *git_connect(int fd[2], const char *url,
 				  const char *prog, int flags)
 {
-	char *host, *path;
+	char *hostandport, *path;
 	struct child_process *conn = &no_fork;
 	enum protocol protocol;
 	const char **arg;
@@ -667,26 +667,22 @@ struct child_process *git_connect(int fd[2], const char *url,
 	 */
 	signal(SIGCHLD, SIG_DFL);
 
-	protocol = parse_connect_url(url, &host, &path);
+	protocol = parse_connect_url(url, &hostandport, &path);
 	if (flags & CONNECT_DIAG_URL) {
 		printf("Diag: url=%s\n", url ? url : "NULL");
 		printf("Diag: protocol=%s\n", prot_name(protocol));
-		printf("Diag: hostandport=%s\n", host ? host : "NULL");
+		printf("Diag: hostandport=%s\n", hostandport ? hostandport : "NULL");
 		printf("Diag: path=%s\n", path ? path : "NULL");
-		free(host);
-		free(path);
-		return NULL;
-	}
-
-	if (protocol == PROTO_GIT) {
+		conn = NULL;
+	} else if (protocol == PROTO_GIT) {
 		/* These underlying connection commands die() if they
 		 * cannot connect.
 		 */
-		char *target_host = xstrdup(host);
-		if (git_use_proxy(host))
-			conn = git_proxy_connect(fd, host);
+		char *target_host = xstrdup(hostandport);
+		if (git_use_proxy(hostandport))
+			conn = git_proxy_connect(fd, hostandport);
 		else
-			git_tcp_connect(fd, host, flags);
+			git_tcp_connect(fd, hostandport, flags);
 		/*
 		 * Separate original protocol components prog and path
 		 * from extended host header with a NUL byte.
@@ -699,54 +695,50 @@ struct child_process *git_connect(int fd[2], const char *url,
 			     prog, path, 0,
 			     target_host, 0);
 		free(target_host);
-		free(host);
-		free(path);
-		return conn;
-	}
-
-	conn = xcalloc(1, sizeof(*conn));
-
-	strbuf_addstr(&cmd, prog);
-	strbuf_addch(&cmd, ' ');
-	sq_quote_buf(&cmd, path);
-
-	conn->in = conn->out = -1;
-	conn->argv = arg = xcalloc(7, sizeof(*arg));
-	if (protocol == PROTO_SSH) {
-		const char *ssh = getenv("GIT_SSH");
-		int putty = ssh && strcasestr(ssh, "plink");
-		char *ssh_host = host; /* keep host for the free() below */
-		const char *port = NULL;
-		get_host_and_port(&ssh_host, &port);
-		port = get_port_numeric(port);
-
-		if (!ssh) ssh = "ssh";
-
-		*arg++ = ssh;
-		if (putty && !strcasestr(ssh, "tortoiseplink"))
-			*arg++ = "-batch";
-		if (port) {
-			/* P is for PuTTY, p is for OpenSSH */
-			*arg++ = putty ? "-P" : "-p";
-			*arg++ = port;
+	} else {
+		conn = xcalloc(1, sizeof(*conn));
+
+		strbuf_addstr(&cmd, prog);
+		strbuf_addch(&cmd, ' ');
+		sq_quote_buf(&cmd, path);
+
+		conn->in = conn->out = -1;
+		conn->argv = arg = xcalloc(7, sizeof(*arg));
+		if (protocol == PROTO_SSH) {
+			const char *ssh = getenv("GIT_SSH");
+			int putty = ssh && strcasestr(ssh, "plink");
+			char *ssh_host = hostandport;
+			const char *port = NULL;
+			get_host_and_port(&ssh_host, &port);
+			port = get_port_numeric(port);
+
+			if (!ssh) ssh = "ssh";
+
+			*arg++ = ssh;
+			if (putty && !strcasestr(ssh, "tortoiseplink"))
+				*arg++ = "-batch";
+			if (port) {
+				/* P is for PuTTY, p is for OpenSSH */
+				*arg++ = putty ? "-P" : "-p";
+				*arg++ = port;
+			}
+			*arg++ = ssh_host;
+		}	else {
+			/* remove repo-local variables from the environment */
+			conn->env = local_repo_env;
+			conn->use_shell = 1;
 		}
-		*arg++ = ssh_host;
-	}
-	else {
-		/* remove repo-local variables from the environment */
-		conn->env = local_repo_env;
-		conn->use_shell = 1;
-	}
-	*arg++ = cmd.buf;
-	*arg = NULL;
+		*arg++ = cmd.buf;
+		*arg = NULL;
 
-	if (start_command(conn))
-		die("unable to fork");
+		if (start_command(conn))
+			die("unable to fork");
 
-	fd[0] = conn->out; /* read from child's stdout */
-	fd[1] = conn->in;  /* write to child's stdin */
-	strbuf_release(&cmd);
-	free(host);
+		fd[0] = conn->out; /* read from child's stdout */
+		fd[1] = conn->in;  /* write to child's stdin */
+		strbuf_release(&cmd);
+	}
+	free(hostandport);
 	free(path);
 	return conn;
 }
-- 
1.8.5.rc0.23.gaa27064
Message 1 of 1 in “git_connect(): Use common return point”
  1. 10/10 git_connect(): Use common return pointTorsten Bögershausen, Nov 28, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.