{"thread":{"id":"35386","subject":"[PATCH v6 08/10] git_connect(): Refactor the port handling","startedAt":"2013-11-21T20:41:47Z","lastAt":"2013-11-21T20:41:47Z","messageCount":1,"participants":["Torsten Bögershausen"],"isPatch":true,"patchVersion":6,"patchTotal":10},"messages":[{"id":"230911","messageId":"201311212141.48067.tboegi@web.de","threadId":"35386","inReplyTo":null,"subject":"[PATCH v6 08/10] git_connect(): Refactor the port handling","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-11-21T20:41:47Z","receivedAt":"2013-11-21T20:41:47Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Use get_host_and_port() even for ssh.\nRemove the variable port git_connect(), and simplify parse_connect_url()\nUse only one return point in git_connect(), doing the free() and return conn.\n\nt5601 had 2 corner test cases which now pass.\n---\n connect.c             | 47 +++++++++++++----------------------------------\n t/t5500-fetch-pack.sh |  9 +++------\n 2 files changed, 16 insertions(+), 40 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 0cb88b8..3d174c8 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -540,16 +540,13 @@ static struct child_process *git_proxy_connect(int fd[2], char *host)\n \treturn proxy;\n }\n \n-static char *get_port(char *host)\n+static const char *get_port_numeric(const char *p)\n {\n \tchar *end;\n-\tchar *p = strchr(host, ':');\n-\n \tif (p) {\n \t\tlong port = strtol(p + 1, &end, 10);\n \t\tif (end != p + 1 && *end == '\\0' && 0 <= port && port < 65536) {\n-\t\t\t*p = '\\0';\n-\t\t\treturn p+1;\n+\t\t\treturn p;\n \t\t}\n \t}\n \n@@ -561,7 +558,7 @@ static char *get_port(char *host)\n  * The caller must free() the returned strings.\n  */\n static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n-\t\t\t\t       char **ret_port, char **ret_path)\n+\t\t\t\t       char **ret_path)\n {\n \tchar *url;\n \tchar *host, *path;\n@@ -569,7 +566,6 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \tint separator;\n \tenum protocol protocol = PROTO_LOCAL;\n \tint free_path = 0;\n-\tchar *port = NULL;\n \n \tif (is_url(url_orig))\n \t\turl = url_decode(url_orig);\n@@ -588,16 +584,12 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t}\n \n \t/*\n-\t * Don't do destructive transforms with git:// as that\n-\t * protocol code does '[]' unwrapping of its own.\n+\t * Don't do destructive transforms as protocol code does\n+\t * '[]' unwrapping in get_host_and_port()\n \t */\n \tif (host[0] == '[') {\n \t\tend = strchr(host + 1, ']');\n \t\tif (end) {\n-\t\t\tif (protocol != PROTO_GIT) {\n-\t\t\t\t*end = 0;\n-\t\t\t\thost++;\n-\t\t\t}\n \t\t\tend++;\n \t\t} else\n \t\t\tend = host;\n@@ -635,17 +627,7 @@ static enum protocol parse_connect_url(const char *url_orig, char **ret_host,\n \t\t*ptr = '\\0';\n \t}\n \n-\t/*\n-\t * Add support for ssh port: ssh://host.xy:<port>/...\n-\t */\n-\tif (protocol == PROTO_SSH && separator == '/')\n-\t\tport = get_port(end);\n-\n \t*ret_host = xstrdup(host);\n-\tif (port)\n-\t\t*ret_port = xstrdup(port);\n-\telse\n-\t\t*ret_port = NULL;\n \tif (free_path)\n \t\t*ret_path = path;\n \telse\n@@ -673,7 +655,6 @@ struct child_process *git_connect(int fd[2], const char *url,\n \tchar *host, *path;\n \tstruct child_process *conn = &no_fork;\n \tenum protocol protocol;\n-\tchar *port;\n \tconst char **arg;\n \tstruct strbuf cmd = STRBUF_INIT;\n \n@@ -682,18 +663,13 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t */\n \tsignal(SIGCHLD, SIG_DFL);\n \n-\tprotocol = parse_connect_url(url, &host, &port, &path);\n+\tprotocol = parse_connect_url(url, &host, &path);\n \tif (flags & CONNECT_DIAG_URL) {\n \t\tfprintf(stderr, \"Diag: url=%s\\n\", url ? url : \"NULL\");\n \t\tfprintf(stderr, \"Diag: protocol=%s\\n\", prot_name(protocol));\n-\t\tfprintf(stderr, \"Diag: host=%s\", host ? host : \"NULL\");\n-\t\tif (port)\n-\t\t\tfprintf(stderr, \":%s\\n\", port);\n-\t\telse\n-\t\t\tfprintf(stderr, \"\\n\");\n+\t\tfprintf(stderr, \"Diag: host=%s\\n\", host ? host : \"NULL\");\n \t\tfprintf(stderr, \"Diag: path=%s\\n\", path ? path : \"NULL\");\n \t\tfree(host);\n-\t\tfree(port);\n \t\tfree(path);\n \t\treturn NULL;\n \t}\n@@ -720,7 +696,6 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t\t     target_host, 0);\n \t\tfree(target_host);\n \t\tfree(host);\n-\t\tfree(port);\n \t\tfree(path);\n \t\treturn conn;\n \t}\n@@ -736,6 +711,11 @@ struct child_process *git_connect(int fd[2], const char *url,\n \tif (protocol == PROTO_SSH) {\n \t\tconst char *ssh = getenv(\"GIT_SSH\");\n \t\tint putty = ssh && strcasestr(ssh, \"plink\");\n+\t\tchar *ssh_host = host; /* keep host for the free() below */\n+\t\tconst char *port = NULL;\n+\t\tget_host_and_port(&ssh_host, &port);\n+\t\tport = get_port_numeric(port);\n+\n \t\tif (!ssh) ssh = \"ssh\";\n \n \t\t*arg++ = ssh;\n@@ -746,7 +726,7 @@ struct child_process *git_connect(int fd[2], const char *url,\n \t\t\t*arg++ = putty ? \"-P\" : \"-p\";\n \t\t\t*arg++ = port;\n \t\t}\n-\t\t*arg++ = host;\n+\t\t*arg++ = ssh_host;\n \t}\n \telse {\n \t\t/* remove repo-local variables from the environment */\n@@ -763,7 +743,6 @@ struct child_process *git_connect(int fd[2], const char *url,\n \tfd[1] = conn->in;  /* write to child's stdin */\n \tstrbuf_release(&cmd);\n \tfree(host);\n-\tfree(port);\n \tfree(path);\n \treturn conn;\n }\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex ac5b08b..69a2110 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -558,18 +558,16 @@ do\n \t\tfor h in host host:12 [::1] [::1]:23\n \t\tdo\n \t\t\tif $(echo $p | grep ssh >/dev/null 2>/dev/null); then\n-\t\t\t\thh=$(echo $h | tr -d \"[]\")\n \t\t\t\tpp=ssh\n \t\t\telse\n-\t\t\t\thh=$h\n \t\t\t\tpp=$p\n \t\t\tfi\n \t\t\ttest_expect_success \"fetch-pack --diag-url $p://$h/$r\" '\n-\t\t\t\tcheck_prot_host_path $p://$h/$r $pp \"$hh\" \"/$r\"\n+\t\t\t\tcheck_prot_host_path $p://$h/$r $pp \"$h\" \"/$r\"\n \t\t\t'\n \t\t\t# \"/~\" -> \"~\" conversion\n \t\t\ttest_expect_success \"fetch-pack --diag-url $p://$h/~$r\" '\n-\t\t\t\tcheck_prot_host_path $p://$h/~$r $pp \"$hh\" \"~$r\"\n+\t\t\t\tcheck_prot_host_path $p://$h/~$r $pp \"$h\" \"~$r\"\n \t\t\t'\n \t\tdone\n \tdone\n@@ -600,13 +598,12 @@ do\n \tp=ssh\n \tfor h in host [::1]\n \tdo\n-\t\thh=$(echo $h | tr -d \"[]\")\n \t\ttest_expect_success \"fetch-pack --diag-url $h:$r\" '\n \t\t\tcheck_prot_path $h:$r $p \"$r\"\n \t\t'\n \t\t# No \"/~\" -> \"~\" conversion\n \t\ttest_expect_success \"fetch-pack --diag-url $h:/~$r\" '\n-\t\t\tcheck_prot_host_path $h:/~$r $p \"$hh\" \"/~$r\"\n+\t\t\tcheck_prot_host_path $h:/~$r $p \"$h\" \"/~$r\"\n \t\t'\n \tdone\n done\n-- \n1.8.4.457.g424cb08\n"}]}