{"thread":{"id":"22654","subject":"[PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","startedAt":"2010-02-14T21:27:40Z","lastAt":"2010-02-15T21:11:56Z","messageCount":12,"participants":["Michael Lukashov","Tay Ray Chuan","Jeff King","Junio C Hamano","Ilari Liusvaara","Larry D'Anna","Daniel Barkalow","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"134570","messageId":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","threadId":"22654","inReplyTo":null,"subject":"[PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-14T21:27:40Z","receivedAt":"2010-02-14T21:27:40Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"The following functions are duplicated:\n\n  verify_remote_names\n  update_tracking_ref\n  print_ref_status\n  status_abbrev\n  print_ok_ref_status\n  print_one_push_status\n  refs_pushed\n  print_push_status\n\nSigned-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n---\n builtin-send-pack.c |   89 ++++++++++++++----------\n send-pack.h         |   20 +++++\n transport.c         |  196 ---------------------------------------------------\n 3 files changed, 72 insertions(+), 233 deletions(-)\n\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 76c7206..616811a 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -169,7 +169,7 @@ static int receive_status(int in, struct ref *refs)\n \treturn ret;\n }\n \n-static void update_tracking_ref(struct remote *remote, struct ref *ref)\n+void update_tracking_ref(struct remote *remote, struct ref *ref, int verbose)\n {\n \tstruct refspec rs;\n \n@@ -180,7 +180,7 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \trs.dst = NULL;\n \n \tif (!remote_find_tracking(remote, &rs)) {\n-\t\tif (args.verbose)\n+\t\tif (verbose)\n \t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n \t\tif (ref->deletion) {\n \t\t\tdelete_ref(rs.dst, NULL, 0);\n@@ -191,37 +191,47 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n \t}\n }\n \n-#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n-\n-static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg)\n+void print_ref_status(char flag, const char *summary, struct ref *to,\n+\t\t  struct ref *from, const char *msg, int porcelain)\n {\n-\tfprintf(stderr, \" %c %-*s \", flag, SUMMARY_WIDTH, summary);\n-\tif (from)\n-\t\tfprintf(stderr, \"%s -> %s\", prettify_refname(from->name), prettify_refname(to->name));\n-\telse\n-\t\tfputs(prettify_refname(to->name), stderr);\n-\tif (msg) {\n-\t\tfputs(\" (\", stderr);\n-\t\tfputs(msg, stderr);\n-\t\tfputc(')', stderr);\n+\tif (porcelain) {\n+\t\tif (from)\n+\t\t\tfprintf(stdout, \"%c\\t%s:%s\\t\", flag, from->name, to->name);\n+\t\telse\n+\t\t\tfprintf(stdout, \"%c\\t:%s\\t\", flag, to->name);\n+\t\tif (msg)\n+\t\t\tfprintf(stdout, \"%s (%s)\\n\", summary, msg);\n+\t\telse\n+\t\t\tfprintf(stdout, \"%s\\n\", summary);\n+\t} else {\n+\t\tfprintf(stderr, \" %c %-*s \", flag, SUMMARY_WIDTH, summary);\n+\t\tif (from)\n+\t\t\tfprintf(stderr, \"%s -> %s\", prettify_refname(from->name), prettify_refname(to->name));\n+\t\telse\n+\t\t\tfputs(prettify_refname(to->name), stderr);\n+\t\tif (msg) {\n+\t\t\tfputs(\" (\", stderr);\n+\t\t\tfputs(msg, stderr);\n+\t\t\tfputc(')', stderr);\n+\t\t}\n+\t\tfputc('\\n', stderr);\n \t}\n-\tfputc('\\n', stderr);\n }\n \n-static const char *status_abbrev(unsigned char sha1[20])\n+const char *status_abbrev(unsigned char sha1[20])\n {\n \treturn find_unique_abbrev(sha1, DEFAULT_ABBREV);\n }\n \n-static void print_ok_ref_status(struct ref *ref)\n+void print_ok_ref_status(struct ref *ref, int porcelain)\n {\n \tif (ref->deletion)\n-\t\tprint_ref_status('-', \"[deleted]\", ref, NULL, NULL);\n+\t\tprint_ref_status('-', \"[deleted]\", ref, NULL, NULL, porcelain);\n \telse if (is_null_sha1(ref->old_sha1))\n \t\tprint_ref_status('*',\n \t\t\t(!prefixcmp(ref->name, \"refs/tags/\") ? \"[new tag]\" :\n \t\t\t  \"[new branch]\"),\n-\t\t\tref, ref->peer_ref, NULL);\n+\t\t\tref, ref->peer_ref, NULL, porcelain);\n \telse {\n \t\tchar quickref[84];\n \t\tchar type;\n@@ -239,73 +249,77 @@ static void print_ok_ref_status(struct ref *ref)\n \t\t}\n \t\tstrcat(quickref, status_abbrev(ref->new_sha1));\n \n-\t\tprint_ref_status(type, quickref, ref, ref->peer_ref, msg);\n+\t\tprint_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);\n \t}\n }\n \n-static int print_one_push_status(struct ref *ref, const char *dest, int count)\n+int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)\n {\n \tif (!count)\n \t\tfprintf(stderr, \"To %s\\n\", dest);\n \n \tswitch(ref->status) {\n \tcase REF_STATUS_NONE:\n-\t\tprint_ref_status('X', \"[no match]\", ref, NULL, NULL);\n+\t\tprint_ref_status('X', \"[no match]\", ref, NULL, NULL, porcelain);\n \t\tbreak;\n \tcase REF_STATUS_REJECT_NODELETE:\n \t\tprint_ref_status('!', \"[rejected]\", ref, NULL,\n-\t\t\t\t\"remote does not support deleting refs\");\n+\t\t\t\t \"remote does not support deleting refs\", porcelain);\n \t\tbreak;\n \tcase REF_STATUS_UPTODATE:\n \t\tprint_ref_status('=', \"[up to date]\", ref,\n-\t\t\t\tref->peer_ref, NULL);\n+\t\t\t\t ref->peer_ref, NULL, porcelain);\n \t\tbreak;\n \tcase REF_STATUS_REJECT_NONFASTFORWARD:\n \t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n-\t\t\t\t\"non-fast-forward\");\n+\t\t\t\t \"non-fast-forward\", porcelain);\n \t\tbreak;\n \tcase REF_STATUS_REMOTE_REJECT:\n \t\tprint_ref_status('!', \"[remote rejected]\", ref,\n \t\t\t\tref->deletion ? NULL : ref->peer_ref,\n-\t\t\t\tref->remote_status);\n+\t\t\t\t\t\t ref->remote_status, porcelain);\n \t\tbreak;\n \tcase REF_STATUS_EXPECTING_REPORT:\n \t\tprint_ref_status('!', \"[remote failure]\", ref,\n \t\t\t\tref->deletion ? NULL : ref->peer_ref,\n-\t\t\t\t\"remote failed to report status\");\n+\t\t\t\t\"remote failed to report status\", porcelain);\n \t\tbreak;\n \tcase REF_STATUS_OK:\n-\t\tprint_ok_ref_status(ref);\n+\t\tprint_ok_ref_status(ref, porcelain);\n \t\tbreak;\n \t}\n \n \treturn 1;\n }\n \n-static void print_push_status(const char *dest, struct ref *refs)\n+void print_push_status(const char *dest, struct ref *refs,\n+\t\t  int verbose, int porcelain, int *nonfastforward)\n {\n \tstruct ref *ref;\n \tint n = 0;\n \n-\tif (args.verbose) {\n+\tif (verbose) {\n \t\tfor (ref = refs; ref; ref = ref->next)\n \t\t\tif (ref->status == REF_STATUS_UPTODATE)\n-\t\t\t\tn += print_one_push_status(ref, dest, n);\n+\t\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n \t}\n \n \tfor (ref = refs; ref; ref = ref->next)\n \t\tif (ref->status == REF_STATUS_OK)\n-\t\t\tn += print_one_push_status(ref, dest, n);\n+\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n \n+\t*nonfastforward = 0;\n \tfor (ref = refs; ref; ref = ref->next) {\n \t\tif (ref->status != REF_STATUS_NONE &&\n \t\t    ref->status != REF_STATUS_UPTODATE &&\n \t\t    ref->status != REF_STATUS_OK)\n-\t\t\tn += print_one_push_status(ref, dest, n);\n+\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n+\t\tif (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)\n+\t\t\t*nonfastforward = 1;\n \t}\n }\n \n-static int refs_pushed(struct ref *ref)\n+int refs_pushed(struct ref *ref)\n {\n \tfor (; ref; ref = ref->next) {\n \t\tswitch(ref->status) {\n@@ -489,7 +503,7 @@ int send_pack(struct send_pack_args *args,\n \treturn 0;\n }\n \n-static void verify_remote_names(int nr_heads, const char **heads)\n+void verify_remote_names(int nr_heads, const char **heads)\n {\n \tint i;\n \n@@ -536,6 +550,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tint send_all = 0;\n \tconst char *receivepack = \"git-receive-pack\";\n \tint flags;\n+\tint nonfastforward = 0;\n \n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\n@@ -657,12 +672,12 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tret |= finish_connect(conn);\n \n \tif (!helper_status)\n-\t\tprint_push_status(dest, remote_refs);\n+\t\tprint_push_status(dest, remote_refs, args.verbose, 0, &nonfastforward);\n \n \tif (!args.dry_run && remote) {\n \t\tstruct ref *ref;\n \t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\tupdate_tracking_ref(remote, ref);\n+\t\t\tupdate_tracking_ref(remote, ref, args.verbose);\n \t}\n \n \tif (!ret && !refs_pushed(remote_refs))\ndiff --git a/send-pack.h b/send-pack.h\nindex 28141ac..decc23c 100644\n--- a/send-pack.h\n+++ b/send-pack.h\n@@ -16,4 +16,24 @@ int send_pack(struct send_pack_args *args,\n \t      int fd[], struct child_process *conn,\n \t      struct ref *remote_refs, struct extra_have_objects *extra_have);\n \n+void verify_remote_names(int nr_heads, const char **heads);\n+\n+void update_tracking_ref(struct remote *remote, struct ref *ref, int verbose);\n+\n+#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n+\n+void print_ref_status(char flag, const char *summary, struct ref *to,\n+\t\t  struct ref *from, const char *msg, int porcelain);\n+\n+const char *status_abbrev(unsigned char sha1[20]);\n+\n+void print_ok_ref_status(struct ref *ref, int porcelain);\n+\n+int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain);\n+\n+int refs_pushed(struct ref *ref);\n+\n+void print_push_status(const char *dest, struct ref *refs,\n+\t\t  int verbose, int porcelain, int *nonfastforward);\n+\n #endif\ndiff --git a/transport.c b/transport.c\nindex 3846aac..aace286 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -573,202 +573,6 @@ static int push_had_errors(struct ref *ref)\n \treturn 0;\n }\n \n-static int refs_pushed(struct ref *ref)\n-{\n-\tfor (; ref; ref = ref->next) {\n-\t\tswitch(ref->status) {\n-\t\tcase REF_STATUS_NONE:\n-\t\tcase REF_STATUS_UPTODATE:\n-\t\t\tbreak;\n-\t\tdefault:\n-\t\t\treturn 1;\n-\t\t}\n-\t}\n-\treturn 0;\n-}\n-\n-static void update_tracking_ref(struct remote *remote, struct ref *ref, int verbose)\n-{\n-\tstruct refspec rs;\n-\n-\tif (ref->status != REF_STATUS_OK && ref->status != REF_STATUS_UPTODATE)\n-\t\treturn;\n-\n-\trs.src = ref->name;\n-\trs.dst = NULL;\n-\n-\tif (!remote_find_tracking(remote, &rs)) {\n-\t\tif (verbose)\n-\t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n-\t\tif (ref->deletion) {\n-\t\t\tdelete_ref(rs.dst, NULL, 0);\n-\t\t} else\n-\t\t\tupdate_ref(\"update by push\", rs.dst,\n-\t\t\t\t\tref->new_sha1, NULL, 0, 0);\n-\t\tfree(rs.dst);\n-\t}\n-}\n-\n-#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n-\n-static void print_ref_status(char flag, const char *summary, struct ref *to, struct ref *from, const char *msg, int porcelain)\n-{\n-\tif (porcelain) {\n-\t\tif (from)\n-\t\t\tfprintf(stdout, \"%c\\t%s:%s\\t\", flag, from->name, to->name);\n-\t\telse\n-\t\t\tfprintf(stdout, \"%c\\t:%s\\t\", flag, to->name);\n-\t\tif (msg)\n-\t\t\tfprintf(stdout, \"%s (%s)\\n\", summary, msg);\n-\t\telse\n-\t\t\tfprintf(stdout, \"%s\\n\", summary);\n-\t} else {\n-\t\tfprintf(stderr, \" %c %-*s \", flag, SUMMARY_WIDTH, summary);\n-\t\tif (from)\n-\t\t\tfprintf(stderr, \"%s -> %s\", prettify_refname(from->name), prettify_refname(to->name));\n-\t\telse\n-\t\t\tfputs(prettify_refname(to->name), stderr);\n-\t\tif (msg) {\n-\t\t\tfputs(\" (\", stderr);\n-\t\t\tfputs(msg, stderr);\n-\t\t\tfputc(')', stderr);\n-\t\t}\n-\t\tfputc('\\n', stderr);\n-\t}\n-}\n-\n-static const char *status_abbrev(unsigned char sha1[20])\n-{\n-\treturn find_unique_abbrev(sha1, DEFAULT_ABBREV);\n-}\n-\n-static void print_ok_ref_status(struct ref *ref, int porcelain)\n-{\n-\tif (ref->deletion)\n-\t\tprint_ref_status('-', \"[deleted]\", ref, NULL, NULL, porcelain);\n-\telse if (is_null_sha1(ref->old_sha1))\n-\t\tprint_ref_status('*',\n-\t\t\t(!prefixcmp(ref->name, \"refs/tags/\") ? \"[new tag]\" :\n-\t\t\t\"[new branch]\"),\n-\t\t\tref, ref->peer_ref, NULL, porcelain);\n-\telse {\n-\t\tchar quickref[84];\n-\t\tchar type;\n-\t\tconst char *msg;\n-\n-\t\tstrcpy(quickref, status_abbrev(ref->old_sha1));\n-\t\tif (ref->nonfastforward) {\n-\t\t\tstrcat(quickref, \"...\");\n-\t\t\ttype = '+';\n-\t\t\tmsg = \"forced update\";\n-\t\t} else {\n-\t\t\tstrcat(quickref, \"..\");\n-\t\t\ttype = ' ';\n-\t\t\tmsg = NULL;\n-\t\t}\n-\t\tstrcat(quickref, status_abbrev(ref->new_sha1));\n-\n-\t\tprint_ref_status(type, quickref, ref, ref->peer_ref, msg, porcelain);\n-\t}\n-}\n-\n-static int print_one_push_status(struct ref *ref, const char *dest, int count, int porcelain)\n-{\n-\tif (!count)\n-\t\tfprintf(stderr, \"To %s\\n\", dest);\n-\n-\tswitch(ref->status) {\n-\tcase REF_STATUS_NONE:\n-\t\tprint_ref_status('X', \"[no match]\", ref, NULL, NULL, porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_REJECT_NODELETE:\n-\t\tprint_ref_status('!', \"[rejected]\", ref, NULL,\n-\t\t\t\t\t\t \"remote does not support deleting refs\", porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_UPTODATE:\n-\t\tprint_ref_status('=', \"[up to date]\", ref,\n-\t\t\t\t\t\t ref->peer_ref, NULL, porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_REJECT_NONFASTFORWARD:\n-\t\tprint_ref_status('!', \"[rejected]\", ref, ref->peer_ref,\n-\t\t\t\t\t\t \"non-fast-forward\", porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_REMOTE_REJECT:\n-\t\tprint_ref_status('!', \"[remote rejected]\", ref,\n-\t\t\t\t\t\t ref->deletion ? NULL : ref->peer_ref,\n-\t\t\t\t\t\t ref->remote_status, porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_EXPECTING_REPORT:\n-\t\tprint_ref_status('!', \"[remote failure]\", ref,\n-\t\t\t\t\t\t ref->deletion ? NULL : ref->peer_ref,\n-\t\t\t\t\t\t \"remote failed to report status\", porcelain);\n-\t\tbreak;\n-\tcase REF_STATUS_OK:\n-\t\tprint_ok_ref_status(ref, porcelain);\n-\t\tbreak;\n-\t}\n-\n-\treturn 1;\n-}\n-\n-static void print_push_status(const char *dest, struct ref *refs,\n-\t\t\t      int verbose, int porcelain, int * nonfastforward)\n-{\n-\tstruct ref *ref;\n-\tint n = 0;\n-\n-\tif (verbose) {\n-\t\tfor (ref = refs; ref; ref = ref->next)\n-\t\t\tif (ref->status == REF_STATUS_UPTODATE)\n-\t\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n-\t}\n-\n-\tfor (ref = refs; ref; ref = ref->next)\n-\t\tif (ref->status == REF_STATUS_OK)\n-\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n-\n-\t*nonfastforward = 0;\n-\tfor (ref = refs; ref; ref = ref->next) {\n-\t\tif (ref->status != REF_STATUS_NONE &&\n-\t\t    ref->status != REF_STATUS_UPTODATE &&\n-\t\t    ref->status != REF_STATUS_OK)\n-\t\t\tn += print_one_push_status(ref, dest, n, porcelain);\n-\t\tif (ref->status == REF_STATUS_REJECT_NONFASTFORWARD)\n-\t\t\t*nonfastforward = 1;\n-\t}\n-}\n-\n-static void verify_remote_names(int nr_heads, const char **heads)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < nr_heads; i++) {\n-\t\tconst char *local = heads[i];\n-\t\tconst char *remote = strrchr(heads[i], ':');\n-\n-\t\tif (*local == '+')\n-\t\t\tlocal++;\n-\n-\t\t/* A matching refspec is okay.  */\n-\t\tif (remote == local && remote[1] == '\\0')\n-\t\t\tcontinue;\n-\n-\t\tremote = remote ? (remote + 1) : local;\n-\t\tswitch (check_ref_format(remote)) {\n-\t\tcase 0: /* ok */\n-\t\tcase CHECK_REF_FORMAT_ONELEVEL:\n-\t\t\t/* ok but a single level -- that is fine for\n-\t\t\t * a match pattern.\n-\t\t\t */\n-\t\tcase CHECK_REF_FORMAT_WILDCARD:\n-\t\t\t/* ok but ends with a pattern-match character */\n-\t\t\tcontinue;\n-\t\t}\n-\t\tdie(\"remote part of refspec is not a valid name in %s\",\n-\t\t    heads[i]);\n-\t}\n-}\n-\n static int git_transport_push(struct transport *transport, struct ref *remote_refs, int flags)\n {\n \tstruct git_transport_data *data = transport->data;\n-- \n1.7.0.1571.g856c2\n"},{"id":"134574","messageId":"1266182863-5048-2-git-send-email-michael.lukashov@gmail.com","threadId":"22654","inReplyTo":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-14T21:27:41Z","receivedAt":"2010-02-14T21:27:41Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"The following functions:\n\n  git_tcp_connect_sock\n  git_tcp_connect_sock,\n  git_proxy_connect\n\nhave common block of code, which is moved to get_host_and_port\n\nSigned-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n---\n connect.c |   83 +++++++++++++++++++++---------------------------------------\n 1 files changed, 29 insertions(+), 54 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex 20054e4..616b312 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -152,6 +152,30 @@ static enum protocol get_protocol(const char *name)\n #define STR_(s)\t# s\n #define STR(s)\tSTR_(s)\n \n+static void get_host_and_port(char **host, const char **port, int set_port_none)\n+{\n+\tchar *colon, *end;\n+\n+\tif (*host[0] == '[') {\n+\t\tend = strchr(*host + 1, ']');\n+\t\tif (end) {\n+\t\t\t*end = 0;\n+\t\t\tend++;\n+\t\t\t(*host)++;\n+\t\t} else\n+\t\t\tend = *host;\n+\t} else\n+\t\tend = *host;\n+\tcolon = strchr(end, ':');\n+\n+\tif (colon) {\n+\t\t*colon = 0;\n+\t\t*port = colon + 1;\n+\t\tif (set_port_none && !**port)\n+\t\t\t*port = \"<none>\";\n+\t}\n+}\n+\n #ifndef NO_IPV6\n \n static const char *ai_name(const struct addrinfo *ai)\n@@ -170,30 +194,12 @@ static const char *ai_name(const struct addrinfo *ai)\n static int git_tcp_connect_sock(char *host, int flags)\n {\n \tint sockfd = -1, saved_errno = 0;\n-\tchar *colon, *end;\n \tconst char *port = STR(DEFAULT_GIT_PORT);\n \tstruct addrinfo hints, *ai0, *ai;\n \tint gai;\n \tint cnt = 0;\n \n-\tif (host[0] == '[') {\n-\t\tend = strchr(host + 1, ']');\n-\t\tif (end) {\n-\t\t\t*end = 0;\n-\t\t\tend++;\n-\t\t\thost++;\n-\t\t} else\n-\t\t\tend = host;\n-\t} else\n-\t\tend = host;\n-\tcolon = strchr(end, ':');\n-\n-\tif (colon) {\n-\t\t*colon = 0;\n-\t\tport = colon + 1;\n-\t\tif (!*port)\n-\t\t\tport = \"<none>\";\n-\t}\n+\tget_host_and_port(&host, &port, 1);\n \n \tmemset(&hints, 0, sizeof(hints));\n \thints.ai_socktype = SOCK_STREAM;\n@@ -251,30 +257,15 @@ static int git_tcp_connect_sock(char *host, int flags)\n static int git_tcp_connect_sock(char *host, int flags)\n {\n \tint sockfd = -1, saved_errno = 0;\n-\tchar *colon, *end;\n-\tchar *port = STR(DEFAULT_GIT_PORT), *ep;\n+\tconst char *port = STR(DEFAULT_GIT_PORT);\n+\tchar *ep;\n \tstruct hostent *he;\n \tstruct sockaddr_in sa;\n \tchar **ap;\n \tunsigned int nport;\n \tint cnt;\n \n-\tif (host[0] == '[') {\n-\t\tend = strchr(host + 1, ']');\n-\t\tif (end) {\n-\t\t\t*end = 0;\n-\t\t\tend++;\n-\t\t\thost++;\n-\t\t} else\n-\t\t\tend = host;\n-\t} else\n-\t\tend = host;\n-\tcolon = strchr(end, ':');\n-\n-\tif (colon) {\n-\t\t*colon = 0;\n-\t\tport = colon + 1;\n-\t}\n+\tget_host_and_port(&host, &port, 0);\n \n \tif (flags & CONNECT_VERBOSE)\n \t\tfprintf(stderr, \"Looking up %s ... \", host);\n@@ -406,26 +397,10 @@ static int git_use_proxy(const char *host)\n static void git_proxy_connect(int fd[2], char *host)\n {\n \tconst char *port = STR(DEFAULT_GIT_PORT);\n-\tchar *colon, *end;\n \tconst char *argv[4];\n \tstruct child_process proxy;\n \n-\tif (host[0] == '[') {\n-\t\tend = strchr(host + 1, ']');\n-\t\tif (end) {\n-\t\t\t*end = 0;\n-\t\t\tend++;\n-\t\t\thost++;\n-\t\t} else\n-\t\t\tend = host;\n-\t} else\n-\t\tend = host;\n-\tcolon = strchr(end, ':');\n-\n-\tif (colon) {\n-\t\t*colon = 0;\n-\t\tport = colon + 1;\n-\t}\n+\tget_host_and_port(&host, &port, 0);\n \n \targv[0] = git_proxy_command;\n \targv[1] = host;\n-- \n1.7.0.1571.g856c2\n"},{"id":"134572","messageId":"1266182863-5048-3-git-send-email-michael.lukashov@gmail.com","threadId":"22654","inReplyTo":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH 3/4] Refactoring: move duplicated code from builtin-pack-objects.c and fast-import.c to object.c","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-14T21:27:42Z","receivedAt":"2010-02-14T21:27:42Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"The following functions are duplicated:\n\n  encode_header\n\nSigned-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n---\n builtin-pack-objects.c |   27 ---------------------------\n fast-import.c          |   23 -----------------------\n object.c               |   20 ++++++++++++++++++++\n object.h               |    9 +++++++++\n 4 files changed, 29 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin-pack-objects.c b/builtin-pack-objects.c\nindex e1d3adf..80bbcd2 100644\n--- a/builtin-pack-objects.c\n+++ b/builtin-pack-objects.c\n@@ -155,33 +155,6 @@ static unsigned long do_compress(void **pptr, unsigned long size)\n }\n \n /*\n- * The per-object header is a pretty dense thing, which is\n- *  - first byte: low four bits are \"size\", then three bits of \"type\",\n- *    and the high bit is \"size continues\".\n- *  - each byte afterwards: low seven bits are size continuation,\n- *    with the high bit being \"size continues\"\n- */\n-static int encode_header(enum object_type type, unsigned long size, unsigned char *hdr)\n-{\n-\tint n = 1;\n-\tunsigned char c;\n-\n-\tif (type < OBJ_COMMIT || type > OBJ_REF_DELTA)\n-\t\tdie(\"bad type %d\", type);\n-\n-\tc = (type << 4) | (size & 15);\n-\tsize >>= 4;\n-\twhile (size) {\n-\t\t*hdr++ = c | 0x80;\n-\t\tc = size & 0x7f;\n-\t\tsize >>= 7;\n-\t\tn++;\n-\t}\n-\t*hdr = c;\n-\treturn n;\n-}\n-\n-/*\n  * we are going to reuse the existing object data as is.  make\n  * sure it is not corrupt.\n  */\ndiff --git a/fast-import.c b/fast-import.c\nindex b477dc6..f983338 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1013,29 +1013,6 @@ static void cycle_packfile(void)\n \tstart_packfile();\n }\n \n-static size_t encode_header(\n-\tenum object_type type,\n-\tuintmax_t size,\n-\tunsigned char *hdr)\n-{\n-\tint n = 1;\n-\tunsigned char c;\n-\n-\tif (type < OBJ_COMMIT || type > OBJ_REF_DELTA)\n-\t\tdie(\"bad type %d\", type);\n-\n-\tc = (type << 4) | (size & 15);\n-\tsize >>= 4;\n-\twhile (size) {\n-\t\t*hdr++ = c | 0x80;\n-\t\tc = size & 0x7f;\n-\t\tsize >>= 7;\n-\t\tn++;\n-\t}\n-\t*hdr = c;\n-\treturn n;\n-}\n-\n static int store_object(\n \tenum object_type type,\n \tstruct strbuf *dat,\ndiff --git a/object.c b/object.c\nindex 3ca92c4..a06ad01 100644\n--- a/object.c\n+++ b/object.c\n@@ -268,3 +268,23 @@ void object_array_remove_duplicates(struct object_array *array)\n \t\tarray->nr = dst;\n \t}\n }\n+\n+int encode_header(enum object_type type, uintmax_t size, unsigned char *hdr)\n+{\n+\tint n = 1;\n+\tunsigned char c;\n+\n+\tif (type < OBJ_COMMIT || type > OBJ_REF_DELTA)\n+\t\tdie(\"bad type %d\", type);\n+\n+\tc = (type << 4) | (size & 15);\n+\tsize >>= 4;\n+\twhile (size) {\n+\t\t*hdr++ = c | 0x80;\n+\t\tc = size & 0x7f;\n+\t\tsize >>= 7;\n+\t\tn++;\n+\t}\n+\t*hdr = c;\n+\treturn n;\n+}\ndiff --git a/object.h b/object.h\nindex 82877c8..f5a5c77 100644\n--- a/object.h\n+++ b/object.h\n@@ -79,4 +79,13 @@ void add_object_array(struct object *obj, const char *name, struct object_array\n void add_object_array_with_mode(struct object *obj, const char *name, struct object_array *array, unsigned mode);\n void object_array_remove_duplicates(struct object_array *);\n \n+/*\n+ * The per-object header is a pretty dense thing, which is\n+ *  - first byte: low four bits are \"size\", then three bits of \"type\",\n+ *    and the high bit is \"size continues\".\n+ *  - each byte afterwards: low seven bits are size continuation,\n+ *    with the high bit being \"size continues\"\n+ */\n+int encode_header(enum object_type type, uintmax_t size, unsigned char *hdr);\n+\n #endif /* OBJECT_H */\n-- \n1.7.0.1571.g856c2\n"},{"id":"134571","messageId":"1266182863-5048-4-git-send-email-michael.lukashov@gmail.com","threadId":"22654","inReplyTo":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH 4/4] Refactoring: remove duplicated code from builtin-checkout.c and merge-recursive.c","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-14T21:27:43Z","receivedAt":"2010-02-14T21:27:43Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"The following functions are duplicated:\n\n  fill_mm\n\nSigned-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n---\n builtin-checkout.c |   18 ------------------\n merge-recursive.c  |    2 +-\n merge-recursive.h  |    3 +++\n 3 files changed, 4 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin-checkout.c b/builtin-checkout.c\nindex 5277817..e53e857 100644\n--- a/builtin-checkout.c\n+++ b/builtin-checkout.c\n@@ -128,24 +128,6 @@ static int checkout_stage(int stage, struct cache_entry *ce, int pos,\n \t\t     (stage == 2) ? \"our\" : \"their\");\n }\n \n-/* NEEDSWORK: share with merge-recursive */\n-static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n-{\n-\tunsigned long size;\n-\tenum object_type type;\n-\n-\tif (!hashcmp(sha1, null_sha1)) {\n-\t\tmm->ptr = xstrdup(\"\");\n-\t\tmm->size = 0;\n-\t\treturn;\n-\t}\n-\n-\tmm->ptr = read_sha1_file(sha1, &type, &size);\n-\tif (!mm->ptr || type != OBJ_BLOB)\n-\t\tdie(\"unable to read blob object %s\", sha1_to_hex(sha1));\n-\tmm->size = size;\n-}\n-\n static int checkout_merged(int pos, struct checkout *state)\n {\n \tstruct cache_entry *ce = active_cache[pos];\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex cb53b01..5999ae2 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -599,7 +599,7 @@ struct merge_file_info\n \t\t merge:1;\n };\n \n-static void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n+void fill_mm(const unsigned char *sha1, mmfile_t *mm)\n {\n \tunsigned long size;\n \tenum object_type type;\ndiff --git a/merge-recursive.h b/merge-recursive.h\nindex be8410a..ccc4002 100644\n--- a/merge-recursive.h\n+++ b/merge-recursive.h\n@@ -2,6 +2,7 @@\n #define MERGE_RECURSIVE_H\n \n #include \"string-list.h\"\n+#include \"xdiff/xdiff.h\"\n \n struct merge_options {\n \tconst char *branch1;\n@@ -53,4 +54,6 @@ int merge_recursive_generic(struct merge_options *o,\n void init_merge_options(struct merge_options *o);\n struct tree *write_tree_from_memory(struct merge_options *o);\n \n+void fill_mm(const unsigned char *sha1, mmfile_t *mm);\n+\n #endif\n-- \n1.7.0.1571.g856c2\n"},{"id":"134595","messageId":"be6fef0d1002141929x1c0f48eekb7112463110cd275@mail.gmail.com","threadId":"22654","inReplyTo":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-15T03:29:22Z","receivedAt":"2010-02-15T03:29:22Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, Feb 15, 2010 at 5:27 AM, Michael Lukashov\n<michael.lukashov@gmail.com> wrote:\n> The following functions are duplicated:\n>\n>  verify_remote_names\n>  update_tracking_ref\n>  print_ref_status\n>  status_abbrev\n>  print_ok_ref_status\n>  print_one_push_status\n>  refs_pushed\n>  print_push_status\n>\n> Signed-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n\nstrictly speaking, the implementation for these functions are\ndifferent. Perhaps you could advertise in the commit message that some\nof the functions from builtin-send-pack.c learnt porcelain, even\nthough it's always off (0).\n\n> diff --git a/builtin-send-pack.c b/builtin-send-pack.c\n> index 76c7206..616811a 100644\n> --- a/builtin-send-pack.c\n> +++ b/builtin-send-pack.c\n> [snip]\n> @@ -191,37 +191,47 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)\n>        }\n>  }\n>\n> -#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n\nhmm, since this is only used internally by print_ref_status, can't\nthis stay here rather than being made public in send-pack.h?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134612","messageId":"20100215052853.GJ3336@coredump.intra.peff.net","threadId":"22654","inReplyTo":"1266182863-5048-1-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-15T05:28:53Z","receivedAt":"2010-02-15T05:28:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 14, 2010 at 09:27:40PM +0000, Michael Lukashov wrote:\n\n> The following functions are duplicated:\n> \n>   verify_remote_names\n>   update_tracking_ref\n>   print_ref_status\n>   status_abbrev\n>   print_ok_ref_status\n>   print_one_push_status\n>   refs_pushed\n>   print_push_status\n> \n> Signed-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n> ---\n>  builtin-send-pack.c |   89 ++++++++++++++----------\n>  send-pack.h         |   20 +++++\n>  transport.c         |  196 ---------------------------------------------------\n\nI think this is backwards. The versions in send-pack were there first,\nand then were ported to transport.c so that other transports could\nbenefit from them. And that is where they should ultimately be.\n\nI can't remember the exact details of why the originals were not\nremoved, though (I think I complained about it once before, and there\nwas some technical reason, but I don't recall now). Daniel (cc'd) might\nremember more.\n\n-Peff\n"},{"id":"134619","messageId":"7v7hqfknwz.fsf@alter.siamese.dyndns.org","threadId":"22654","inReplyTo":"20100215052853.GJ3336@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-15T06:34:20Z","receivedAt":"2010-02-15T06:34:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, Feb 14, 2010 at 09:27:40PM +0000, Michael Lukashov wrote:\n>\n>> The following functions are duplicated:\n>> \n>>   verify_remote_names\n>>   update_tracking_ref\n>>   print_ref_status\n>>   status_abbrev\n>>   print_ok_ref_status\n>>   print_one_push_status\n>>   refs_pushed\n>>   print_push_status\n>> \n>> Signed-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n>> ---\n>>  builtin-send-pack.c |   89 ++++++++++++++----------\n>>  send-pack.h         |   20 +++++\n>>  transport.c         |  196 ---------------------------------------------------\n>\n> I think this is backwards. The versions in send-pack were there first,\n> and then were ported to transport.c so that other transports could\n> benefit from them. And that is where they should ultimately be.\n>\n> I can't remember the exact details of why the originals were not\n> removed, though (I think I complained about it once before, and there\n> was some technical reason, but I don't recall now). Daniel (cc'd) might\n> remember more.\n\nAlso the names of these functions probably need to be made more specific\nso that people not so familiar with the transport code can tell that they\nare from \"transport\" family.  The names didn't matter much while they were\nfile scope static, but this series changes that.\n"},{"id":"134626","messageId":"20100215075514.GB5347@coredump.intra.peff.net","threadId":"22654","inReplyTo":"7v7hqfknwz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-15T07:55:15Z","receivedAt":"2010-02-15T07:55:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:\n\n> > I can't remember the exact details of why the originals were not\n> > removed, though (I think I complained about it once before, and there\n> > was some technical reason, but I don't recall now). Daniel (cc'd) might\n> > remember more.\n> \n> Also the names of these functions probably need to be made more specific\n> so that people not so familiar with the transport code can tell that they\n> are from \"transport\" family.  The names didn't matter much while they were\n> file scope static, but this series changes that.\n\nActually, I wonder if we can simply get rid of some of the calls in\nsend-pack. I think that the code in send-pack isn't even called anymore\nvia \"git push\"; it only gets called when you call send-pack directly.\nAnd arguably send-pack as plumbing shouldn't be generating all sorts of\nuser-facing output. But it is a behavior change. I wonder if anybody\nactually calls send-pack directly anymore. It seems like even scripts\nuse \"git push\" because of the transport agnosticism.\n\n-Peff\n"},{"id":"134632","messageId":"20100215084643.GA26012@Knoppix","threadId":"22654","inReplyTo":"20100215075514.GB5347@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-02-15T08:46:43Z","receivedAt":"2010-02-15T08:46:43Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Mon, Feb 15, 2010 at 02:55:15AM -0500, Jeff King wrote:\n> On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:\n> \n> Actually, I wonder if we can simply get rid of some of the calls in\n> send-pack. I think that the code in send-pack isn't even called anymore\n> via \"git push\"; it only gets called when you call send-pack directly.\n\nActually, its also seemingly called by git-remote-http(s) (at least it\ncontains references to \"stateless RPC\", which is related to smart HTTP).\n\n> And arguably send-pack as plumbing shouldn't be generating all sorts of\n> user-facing output. But it is a behavior change. I wonder if anybody\n> actually calls send-pack directly anymore. It seems like even scripts\n> use \"git push\" because of the transport agnosticism.\n\nFor non-stateless case, it seems that the only protocols builtin-send-pack\ncan deal with are ssh://, git:// and file://, it can't deal with any\nsort of remote helper, not even one provoding smart transport.\n\n-Ilari\n"},{"id":"134649","messageId":"20100215173041.GA8215@cthulhu","threadId":"22654","inReplyTo":"7v635zj8jr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-15T17:30:41Z","receivedAt":"2010-02-15T17:30:41Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"\nWeird: I only got the Cc for this, git@vger.kernel.org didnt' sent it to me.  It\ndoesn't seem to be on gmane either.\n\n* Junio C Hamano (gitster@pobox.com) [100215 01:51]:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > Jeff King <peff@peff.net> writes:\n> >\n> >>>  builtin-send-pack.c |   89 ++++++++++++++----------\n> >>>  send-pack.h         |   20 +++++\n> >>>  transport.c         |  196 ---------------------------------------------------\n> >>\n> >> I think this is backwards. The versions in send-pack were there first,\n> >> and then were ported to transport.c so that other transports could\n> >> benefit from them. And that is where they should ultimately be.\n> >\n> > Also the names of these functions probably need to be made more specific\n> > so that people not so familiar with the transport code can tell that they\n> > are from \"transport\" family.  The names didn't matter much while they were\n> > file scope static, but this series changes that.\n> \n> Ah, one more thing.  I think this patch touches somewhat overlapping areas\n> the ld/push-porcelain topic in 'pu' touches.\n> \n> I think Peff's \"backwards\" observation is correct (and Daniel can\n> elaborate if he wants).  Once the direction is set on that point, you and\n> Larry probably would need to coordinate to decide how to proceed.  My gut\n> feeling without actually looking at the conflicts is that applying your\n> code consolidation first and then doing the \"porcelain\" rework on top\n> might be a cleaner approach, but you two are in better position to decide\n> on the order, as these are your codes that will be conflicting with each\n> other.\n\nThat sounds good to me.  I'll rebase the porcelain stuff off the next version of\nMichael's series.\n\n          --larry\n"},{"id":"134651","messageId":"alpine.LNX.2.00.1002151250030.14365@iabervon.org","threadId":"22654","inReplyTo":"20100215075514.GB5347@coredump.intra.peff.net","subject":"Re: [PATCH 1/4] Refactoring: remove duplicated code from transport.c and builtin-send-pack.c","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2010-02-15T18:25:35Z","receivedAt":"2010-02-15T18:25:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Mon, 15 Feb 2010, Jeff King wrote:\n\n> On Sun, Feb 14, 2010 at 10:34:20PM -0800, Junio C Hamano wrote:\n> \n> > > I can't remember the exact details of why the originals were not\n> > > removed, though (I think I complained about it once before, and there\n> > > was some technical reason, but I don't recall now). Daniel (cc'd) might\n> > > remember more.\n> > \n> > Also the names of these functions probably need to be made more specific\n> > so that people not so familiar with the transport code can tell that they\n> > are from \"transport\" family.  The names didn't matter much while they were\n> > file scope static, but this series changes that.\n> \n> Actually, I wonder if we can simply get rid of some of the calls in\n> send-pack. I think that the code in send-pack isn't even called anymore\n> via \"git push\"; it only gets called when you call send-pack directly.\n> And arguably send-pack as plumbing shouldn't be generating all sorts of\n> user-facing output. But it is a behavior change. I wonder if anybody\n> actually calls send-pack directly anymore. It seems like even scripts\n> use \"git push\" because of the transport agnosticism.\n\nI think it would probably be better to get rid of send-pack as a separate \ncommand entirely, rather than changing any of its behavior, and make \nremote-curl use a private command that only has the desired behavior, \nwhich is stdio to a local proxy for the remote.\n\nFor that matter, it would likely be worthwhile abstracting the packet_line \ncode such that send-pack (and fetch-pack) could be done in-process without \nthe messages going over a classic packet_line connection to remote-curl \nbefore being sent over HTTP to the actual server.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"134665","messageId":"4B79B89C.1050603@kdbg.org","threadId":"22654","inReplyTo":"1266182863-5048-2-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-02-15T21:11:56Z","receivedAt":"2010-02-15T21:11:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Michael Lukashov schrieb:\n> +static void get_host_and_port(char **host, const char **port, int set_port_none)\n\nMinor nit: The last parameter, set_port_none, is a rather prominent sign \nthat this function mixes policy and functionality. And indeed, this \nimplementation:\n\n> +\tif (colon) {\n> +\t\t*colon = 0;\n> +\t\t*port = colon + 1;\n> +\t\tif (set_port_none && !**port)\n> +\t\t\t*port = \"<none>\";\n> +\t}\n\nproves it. The _functionality_ is to find host and port from a string. The \n_policy_ is to set the port to \"<none>\" if it would otherwise be empty. \nThe callers take care of the _policy_, this function should only care \nabout _functionality_. There's only one call site that wants \"<none>\"; \ndon't move this detail into this function.\n\nOther than that: nice catch.\n\n-- Hannes\n"}]}