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

[PATCH 2/2] git_connect: factor out discovery of the protocol and its parts

From
Johannes Sixt <j6t@kdbg.org>
Date
Nov 5, 2013, 19:39 UTC
Message-ID
<5279496A.2090202@kdbg.org>
In-Reply-To
<52794882.2020108@kdbg.org>

git_connect has grown large due to the many different protocols syntaxes that are supported. Move the part of the function that parses the URL to connect to into a separate function for readability.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Apart from this "simplification", the protocol parsing code is a complete
mystery to me. There is more potential for simplification because
git_proxy_connect() and git_tcp_connect() do their own host+port parsing.
 connect.c | 80 ++++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 27 deletions(-)
diff --git a/connect.c b/connect.c
index 6cc1f8d..a6cf345 100644
--- a/connect.c
+++ b/connect.c
@@ -543,37 +543,20 @@ static char *get_port(char *host)
 	return NULL;
 }
 
-static struct child_process no_fork;
-
 /*
- * 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 (this
- * will hopefully be changed in a libification effort, to return NULL when
- * the connection failed).
+ * Extract protocol and relevant parts from the specified connection URL.
+ * The caller must free() the returned strings.
  */
-struct child_process *git_connect(int fd[2], const char *url_orig,
-				  const char *prog, int flags)
+static enum protocol parse_connect_url(const char *url_orig, char **ret_host,
+				       char **ret_port, char **ret_path)
 {
 	char *url;
 	char *host, *path;
 	char *end;
 	int c;
-	struct child_process *conn = &no_fork;
 	enum protocol protocol = PROTO_LOCAL;
 	int free_path = 0;
 	char *port = NULL;
-	const char **arg;
-	struct strbuf cmd = STRBUF_INIT;
-
-	/* Without this we cannot rely on waitpid() to tell
-	 * what happened to our children.
-	 */
-	signal(SIGCHLD, SIG_DFL);
 
 	if (is_url(url_orig))
 		url = url_decode(url_orig);
@@ -645,6 +628,49 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	if (protocol == PROTO_SSH && host != url)
 		port = get_port(end);
 
+	*ret_host = xstrdup(host);
+	if (port)
+		*ret_port = xstrdup(port);
+	else
+		*ret_port = NULL;
+	if (free_path)
+		*ret_path = path;
+	else
+		*ret_path = xstrdup(path);
+	free(url);
+	return protocol;
+}
+
+static struct child_process no_fork;
+
+/*
+ * 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 (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,
+				  const char *prog, int flags)
+{
+	char *host, *path;
+	struct child_process *conn = &no_fork;
+	enum protocol protocol;
+	char *port;
+	const char **arg;
+	struct strbuf cmd = STRBUF_INIT;
+
+	/* Without this we cannot rely on waitpid() to tell
+	 * what happened to our children.
+	 */
+	signal(SIGCHLD, SIG_DFL);
+
+	protocol = parse_connect_url(url, &host, &port, &path);
+
 	if (protocol == PROTO_GIT) {
 		/* These underlying connection commands die() if they
 		 * cannot connect.
@@ -666,9 +692,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 			     prog, path, 0,
 			     target_host, 0);
 		free(target_host);
-		free(url);
-		if (free_path)
-			free(path);
+		free(host);
+		free(port);
+		free(path);
 		return conn;
 	}
 
@@ -709,9 +735,9 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 	fd[0] = conn->out; /* read from child's stdout */
 	fd[1] = conn->in;  /* write to child's stdin */
 	strbuf_release(&cmd);
-	free(url);
-	if (free_path)
-		free(path);
+	free(host);
+	free(port);
+	free(path);
 	return conn;
 }
 
-- 
1.8.4.33.gd68f7e8
Previous: Johannes SixtNext: Torsten Bögershausen
Message 4 of 7 in “git clone: is an URL local or ssh”
  1. git clone: is an URL local or sshTorsten Bögershausen, Nov 4, 2013
  2. Johannes SixtNov 5, 2013
  3. 1/2 git_connect: remove artificial limit of a remote commandJohannes Sixt, Nov 5, 2013
  4. 2/2 git_connect: factor out discovery of the protocol and its partsJohannes Sixt, Nov 5, 2013
  5. Torsten BögershausenNov 5, 2013
  6. Johannes SixtNov 5, 2013
  7. Torsten BögershausenNov 6, 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.