{"thread":{"id":"22668","subject":"[PATCH v2 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port","startedAt":"2010-02-15T23:26:46Z","lastAt":"2010-02-16T19:41:57Z","messageCount":10,"participants":["Michael Lukashov","Tay Ray Chuan","Larry D'Anna","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"134679","messageId":"1266276411-5796-1-git-send-email-michael.lukashov@gmail.com","threadId":"22668","inReplyTo":null,"subject":"[PATCH v2 0/4] Refactoring: remove duplicated code","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-15T23:26:46Z","receivedAt":"2010-02-15T23:26:46Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"Hi,\n\nHere is the 2nd iteration of my refactoring patches. \n\nMichael Lukashov (4):\n  Refactoring: remove duplicated code from builtin-send-pack.c and\n    transport.c\n  Refactoring: connect.c: move duplicated code to get_host_and_port\n  Refactoring: move duplicated code from builtin-pack-objects.c and\n    fast-import.c to object.c\n  Refactoring: remove duplicated code from builtin-checkout.c and\n    merge-recursive.c\n\n builtin-checkout.c     |   18 -----\n builtin-fetch.c        |   20 +++---\n builtin-pack-objects.c |   27 -------\n builtin-send-pack.c    |  190 +----------------------------------------------\n connect.c              |   83 +++++++--------------\n fast-import.c          |   23 ------\n merge-recursive.c      |    2 +-\n merge-recursive.h      |    3 +\n object.c               |   20 +++++\n object.h               |    9 ++\n transport.c            |   27 +++----\n transport.h            |   11 +++\n 12 files changed, 101 insertions(+), 332 deletions(-)\n"},{"id":"134680","messageId":"1266276411-5796-2-git-send-email-michael.lukashov@gmail.com","threadId":"22668","inReplyTo":"1266276411-5796-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH v2 1/4] Refactoring: remove duplicated code from builtin-send-pack.c and transport.c","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-15T23:26:47Z","receivedAt":"2010-02-15T23:26:47Z","isPatch":true,"sender":{"key":"michael.lukashov@gmail.com","avatar":"https://avatars.githubusercontent.com/u/890439?v=4"},"body":"The following functions are (almost) identical:\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\nMove common versions of these functions to transport.c and rename them,\nas suggested by Jeff King and Junio C Hamano\n\nAlso, move #define SUMMARY_WIDTH to transport.h and rename it TRANSPORT_SUMMARY_WIDTH\nas it is used in builtin-fetch.c and transport.c\n\nSigned-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n---\n builtin-fetch.c     |   20 +++---\n builtin-send-pack.c |  190 ++-------------------------------------------------\n transport.c         |   27 ++++----\n transport.h         |   11 +++\n 4 files changed, 39 insertions(+), 209 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex 8654fa7..d3b9d8a 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -11,6 +11,7 @@\n #include \"run-command.h\"\n #include \"parse-options.h\"\n #include \"sigchain.h\"\n+#include \"transport.h\"\n \n static const char * const builtin_fetch_usage[] = {\n \t\"git fetch [options] [<repository> <refspec>...]\",\n@@ -205,7 +206,6 @@ static int s_update_ref(const char *action,\n \treturn 0;\n }\n \n-#define SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n #define REFCOL_WIDTH  10\n \n static int update_local_ref(struct ref *ref,\n@@ -224,7 +224,7 @@ static int update_local_ref(struct ref *ref,\n \n \tif (!hashcmp(ref->old_sha1, ref->new_sha1)) {\n \t\tif (verbosity > 0)\n-\t\t\tsprintf(display, \"= %-*s %-*s -> %s\", SUMMARY_WIDTH,\n+\t\t\tsprintf(display, \"= %-*s %-*s -> %s\", TRANSPORT_SUMMARY_WIDTH,\n \t\t\t\t\"[up to date]\", REFCOL_WIDTH, remote,\n \t\t\t\tpretty_ref);\n \t\treturn 0;\n@@ -239,7 +239,7 @@ static int update_local_ref(struct ref *ref,\n \t\t * the head, and the old value of the head isn't empty...\n \t\t */\n \t\tsprintf(display, \"! %-*s %-*s -> %s  (can't fetch in current branch)\",\n-\t\t\tSUMMARY_WIDTH, \"[rejected]\", REFCOL_WIDTH, remote,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, \"[rejected]\", REFCOL_WIDTH, remote,\n \t\t\tpretty_ref);\n \t\treturn 1;\n \t}\n@@ -249,7 +249,7 @@ static int update_local_ref(struct ref *ref,\n \t\tint r;\n \t\tr = s_update_ref(\"updating tag\", ref, 0);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : '-',\n-\t\t\tSUMMARY_WIDTH, \"[tag update]\", REFCOL_WIDTH, remote,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, \"[tag update]\", REFCOL_WIDTH, remote,\n \t\t\tpretty_ref, r ? \"  (unable to update local ref)\" : \"\");\n \t\treturn r;\n \t}\n@@ -271,7 +271,7 @@ static int update_local_ref(struct ref *ref,\n \n \t\tr = s_update_ref(msg, ref, 0);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : '*',\n-\t\t\tSUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, what, REFCOL_WIDTH, remote, pretty_ref,\n \t\t\tr ? \"  (unable to update local ref)\" : \"\");\n \t\treturn r;\n \t}\n@@ -284,7 +284,7 @@ static int update_local_ref(struct ref *ref,\n \t\tstrcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));\n \t\tr = s_update_ref(\"fast-forward\", ref, 1);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s%s\", r ? '!' : ' ',\n-\t\t\tSUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n \t\t\tpretty_ref, r ? \"  (unable to update local ref)\" : \"\");\n \t\treturn r;\n \t} else if (force || ref->force) {\n@@ -295,13 +295,13 @@ static int update_local_ref(struct ref *ref,\n \t\tstrcat(quickref, find_unique_abbrev(ref->new_sha1, DEFAULT_ABBREV));\n \t\tr = s_update_ref(\"forced-update\", ref, 1);\n \t\tsprintf(display, \"%c %-*s %-*s -> %s  (%s)\", r ? '!' : '+',\n-\t\t\tSUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, quickref, REFCOL_WIDTH, remote,\n \t\t\tpretty_ref,\n \t\t\tr ? \"unable to update local ref\" : \"forced update\");\n \t\treturn r;\n \t} else {\n \t\tsprintf(display, \"! %-*s %-*s -> %s  (non-fast-forward)\",\n-\t\t\tSUMMARY_WIDTH, \"[rejected]\", REFCOL_WIDTH, remote,\n+\t\t\tTRANSPORT_SUMMARY_WIDTH, \"[rejected]\", REFCOL_WIDTH, remote,\n \t\t\tpretty_ref);\n \t\treturn 1;\n \t}\n@@ -393,7 +393,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\trc |= update_local_ref(ref, what, note);\n \t\telse\n \t\t\tsprintf(note, \"* %-*s %-*s -> FETCH_HEAD\",\n-\t\t\t\tSUMMARY_WIDTH, *kind ? kind : \"branch\",\n+\t\t\t\tTRANSPORT_SUMMARY_WIDTH, *kind ? kind : \"branch\",\n \t\t\t\t REFCOL_WIDTH, *what ? what : \"HEAD\");\n \t\tif (*note) {\n \t\t\tif (verbosity >= 0 && !shown_url) {\n@@ -514,7 +514,7 @@ static int prune_refs(struct transport *transport, struct ref *ref_map)\n \t\t\tresult |= delete_ref(ref->name, NULL, 0);\n \t\tif (verbosity >= 0) {\n \t\t\tfprintf(stderr, \" x %-*s %-*s -> %s\\n\",\n-\t\t\t\tSUMMARY_WIDTH, \"[deleted]\",\n+\t\t\t\tTRANSPORT_SUMMARY_WIDTH, \"[deleted]\",\n \t\t\t\tREFCOL_WIDTH, \"(none)\", prettify_refname(ref->name));\n \t\t\twarn_dangling_symref(stderr, dangling_msg, ref->name);\n \t\t}\ndiff --git a/builtin-send-pack.c b/builtin-send-pack.c\nindex 76c7206..b6e8948 100644\n--- a/builtin-send-pack.c\n+++ b/builtin-send-pack.c\n@@ -169,156 +169,6 @@ 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-{\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 (args.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)\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-\t}\n-\tfputc('\\n', stderr);\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)\n-{\n-\tif (ref->deletion)\n-\t\tprint_ref_status('-', \"[deleted]\", ref, NULL, NULL);\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-\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);\n-\t}\n-}\n-\n-static int print_one_push_status(struct ref *ref, const char *dest, int count)\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\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\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\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\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\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\tbreak;\n-\tcase REF_STATUS_OK:\n-\t\tprint_ok_ref_status(ref);\n-\t\tbreak;\n-\t}\n-\n-\treturn 1;\n-}\n-\n-static void print_push_status(const char *dest, struct ref *refs)\n-{\n-\tstruct ref *ref;\n-\tint n = 0;\n-\n-\tif (args.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}\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-\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}\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 print_helper_status(struct ref *ref)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n@@ -489,37 +339,6 @@ 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-{\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 int cmd_send_pack(int argc, const char **argv, const char *prefix)\n {\n \tint i, nr_refspecs = 0;\n@@ -536,6 +355,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@@ -628,7 +448,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \tget_remote_heads(fd[0], &remote_refs, 0, NULL, REF_NORMAL,\n \t\t\t &extra_have);\n \n-\tverify_remote_names(nr_refspecs, refspecs);\n+\ttransport_verify_remote_names(nr_refspecs, refspecs);\n \n \tlocal_refs = get_local_heads();\n \n@@ -657,15 +477,15 @@ 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\ttransport_print_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\ttransport_update_tracking_ref(remote, ref, args.verbose);\n \t}\n \n-\tif (!ret && !refs_pushed(remote_refs))\n+\tif (!ret && !transport_refs_pushed(remote_refs))\n \t\tfprintf(stderr, \"Everything up-to-date\\n\");\n \n \treturn ret;\ndiff --git a/transport.c b/transport.c\nindex 3846aac..0924288 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -573,7 +573,7 @@ static int push_had_errors(struct ref *ref)\n \treturn 0;\n }\n \n-static int refs_pushed(struct ref *ref)\n+int transport_refs_pushed(struct ref *ref)\n {\n \tfor (; ref; ref = ref->next) {\n \t\tswitch(ref->status) {\n@@ -587,7 +587,7 @@ static int refs_pushed(struct ref *ref)\n \treturn 0;\n }\n \n-static void update_tracking_ref(struct remote *remote, struct ref *ref, int verbose)\n+void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int verbose)\n {\n \tstruct refspec rs;\n \n@@ -609,9 +609,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref, int verb\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+static 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 \tif (porcelain) {\n \t\tif (from)\n@@ -623,7 +622,7 @@ static void print_ref_status(char flag, const char *summary, struct ref *to, str\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\tfprintf(stderr, \" %c %-*s \", flag, TRANSPORT_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@@ -687,7 +686,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\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\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@@ -711,8 +710,8 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\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+void transport_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@@ -738,7 +737,7 @@ static void print_push_status(const char *dest, struct ref *refs,\n \t}\n }\n \n-static void verify_remote_names(int nr_heads, const char **heads)\n+void transport_verify_remote_names(int nr_heads, const char **heads)\n {\n \tint i;\n \n@@ -1018,7 +1017,7 @@ int transport_push(struct transport *transport,\n \t\t   int *nonfastforward)\n {\n \t*nonfastforward = 0;\n-\tverify_remote_names(refspec_nr, refspec);\n+\ttransport_verify_remote_names(refspec_nr, refspec);\n \n \tif (transport->push) {\n \t\t/* Maybe FIXME. But no important transport uses this case. */\n@@ -1057,7 +1056,7 @@ int transport_push(struct transport *transport,\n \t\tret |= err;\n \n \t\tif (!quiet || err)\n-\t\t\tprint_push_status(transport->url, remote_refs,\n+\t\t\ttransport_print_push_status(transport->url, remote_refs,\n \t\t\t\t\tverbose | porcelain, porcelain,\n \t\t\t\t\tnonfastforward);\n \n@@ -1067,10 +1066,10 @@ int transport_push(struct transport *transport,\n \t\tif (!(flags & TRANSPORT_PUSH_DRY_RUN)) {\n \t\t\tstruct ref *ref;\n \t\t\tfor (ref = remote_refs; ref; ref = ref->next)\n-\t\t\t\tupdate_tracking_ref(transport->remote, ref, verbose);\n+\t\t\t\ttransport_update_tracking_ref(transport->remote, ref, verbose);\n \t\t}\n \n-\t\tif (!quiet && !ret && !refs_pushed(remote_refs))\n+\t\tif (!quiet && !ret && !transport_refs_pushed(remote_refs))\n \t\t\tfprintf(stderr, \"Everything up-to-date\\n\");\n \t\treturn ret;\n \t}\ndiff --git a/transport.h b/transport.h\nindex 7cea5cc..7a9bb57 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -92,6 +92,7 @@ struct transport {\n #define TRANSPORT_PUSH_PORCELAIN 32\n #define TRANSPORT_PUSH_QUIET 64\n #define TRANSPORT_PUSH_SET_UPSTREAM 128\n+#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)\n \n /* Returns a transport suitable for the url */\n struct transport *transport_get(struct remote *, const char *);\n@@ -142,4 +143,14 @@ int transport_connect(struct transport *transport, const char *name,\n /* Transport methods defined outside transport.c */\n int transport_helper_init(struct transport *transport, const char *name);\n \n+/* common methods used by transport.c and builtin-send-pack.c */\n+void transport_verify_remote_names(int nr_heads, const char **heads);\n+\n+void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int verbose);\n+\n+int transport_refs_pushed(struct ref *ref);\n+\n+void transport_print_push_status(const char *dest, struct ref *refs,\n+\t\t  int verbose, int porcelain, int *nonfastforward);\n+\n #endif\n-- \n1.7.0.1571.g856c2\n"},{"id":"134675","messageId":"1266276411-5796-3-git-send-email-michael.lukashov@gmail.com","threadId":"22668","inReplyTo":"1266276411-5796-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH v2 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port","fromName":"Michael Lukashov","fromEmail":"michael.lukashov@gmail.com","sentAt":"2010-02-15T23:26:48Z","receivedAt":"2010-02-15T23:26:48Z","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 (IPV6 version)\n  git_tcp_connect_sock (no IPV6 version),\n  git_proxy_connect\n\nhave common block of code. Move it to a new function '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..cd399f4 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -152,6 +152,28 @@ 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)\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}\n+}\n+\n #ifndef NO_IPV6\n \n static const char *ai_name(const struct addrinfo *ai)\n@@ -170,30 +192,14 @@ 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);\n+\tif (!*port)\n+\t\t*port = \"<none>\";\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);\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);\n \n \targv[0] = git_proxy_command;\n \targv[1] = host;\n-- \n1.7.0.1571.g856c2\n"},{"id":"134677","messageId":"1266276411-5796-4-git-send-email-michael.lukashov@gmail.com","threadId":"22668","inReplyTo":"1266276411-5796-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH v2 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-15T23:26:49Z","receivedAt":"2010-02-15T23:26:49Z","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":"134678","messageId":"1266276411-5796-5-git-send-email-michael.lukashov@gmail.com","threadId":"22668","inReplyTo":"1266276411-5796-1-git-send-email-michael.lukashov@gmail.com","subject":"[PATCH v2 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-15T23:26:50Z","receivedAt":"2010-02-15T23:26:50Z","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":"134689","messageId":"20100216101613.4ce36ee1.rctay89@gmail.com","threadId":"22668","inReplyTo":"1266276411-5796-2-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH v2 1/4] Refactoring: remove duplicated code from builtin-send-pack.c and transport.c","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2010-02-16T02:16:13Z","receivedAt":"2010-02-16T02:16:13Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Hi,\n\nOn Mon, 15 Feb 2010 23:26:47 +0000\nMichael Lukashov <michael.lukashov@gmail.com> wrote:\n\n> The following functions are (almost) identical:\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> Move common versions of these functions to transport.c and rename them,\n> as suggested by Jeff King and Junio C Hamano\n\nthis is misleading. This list should the 4 functions added to\ntransport.h. Some of the functions have been removed entirely from\nbuiltin-send-pack.c and aren't renamed at all (eg. print_ref_status,\nprint_one_push_status).\n\nFor these, you could put them in another list, and say that \"they have\nbeen removed entirely and will not be made public, since they are only\nused internally by print_push_status().\"\n\n> diff --git a/transport.c b/transport.c\n> index 3846aac..0924288 100644\n> --- a/transport.c\n> +++ b/transport.c\n> [snip]\n> @@ -609,9 +609,8 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref, int verb\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> +static 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>  \tif (porcelain) {\n>  \t\tif (from)\n\nUnrelated whitespace change in the method signature.\n\n> @@ -687,7 +686,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\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\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\nUnrelated whitespace change.\n\n> @@ -711,8 +710,8 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i\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> +void transport_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\nUnrelated whitespace change for the second line of the signature.\n\n-- \nCheers,\nRay Chuan\n"},{"id":"134694","messageId":"20100216041004.GA7529@cthulhu","threadId":"22668","inReplyTo":"1266276411-5796-3-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH v2 2/4] Refactoring: connect.c: move duplicated code to get_host_and_port","fromName":"Larry D'Anna","fromEmail":"larry@elder-gods.org","sentAt":"2010-02-16T04:10:04Z","receivedAt":"2010-02-16T04:10:04Z","isPatch":true,"sender":{"key":"larry@elder-gods.org","avatar":"https://avatars.githubusercontent.com/u/3013304?v=4"},"body":"* Michael Lukashov (michael.lukashov@gmail.com) [100215 18:33]:\n> The following functions:\n> \n>   git_tcp_connect_sock (IPV6 version)\n>   git_tcp_connect_sock (no IPV6 version),\n>   git_proxy_connect\n> \n> have common block of code. Move it to a new function 'get_host_and_port'\n> \n> Signed-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n> ---\n>  connect.c |   83 +++++++++++++++++++++---------------------------------------\n>  1 files changed, 29 insertions(+), 54 deletions(-)\n> \n> diff --git a/connect.c b/connect.c\n> index 20054e4..cd399f4 100644\n> --- a/connect.c\n> +++ b/connect.c\n> @@ -152,6 +152,28 @@ 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)\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}\n> +}\n> +\n>  #ifndef NO_IPV6\n>  \n>  static const char *ai_name(const struct addrinfo *ai)\n> @@ -170,30 +192,14 @@ 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);\n> +\tif (!*port)\n> +\t\t*port = \"<none>\";\n\nshouldn't that be 'port = \"none\";'?\n\n          --larry\n"},{"id":"134712","messageId":"20100216072958.GH2169@coredump.intra.peff.net","threadId":"22668","inReplyTo":"1266276411-5796-2-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH v2 1/4] Refactoring: remove duplicated code from builtin-send-pack.c and transport.c","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-02-16T07:29:58Z","receivedAt":"2010-02-16T07:29:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2010 at 11:26:47PM +0000, Michael Lukashov wrote:\n\n> diff --git a/builtin-fetch.c b/builtin-fetch.c\n> index 8654fa7..d3b9d8a 100644\n> --- a/builtin-fetch.c\n> +++ b/builtin-fetch.c\n> [...]\n> @@ -224,7 +224,7 @@ static int update_local_ref(struct ref *ref,\n>  \n>  \tif (!hashcmp(ref->old_sha1, ref->new_sha1)) {\n>  \t\tif (verbosity > 0)\n> -\t\t\tsprintf(display, \"= %-*s %-*s -> %s\", SUMMARY_WIDTH,\n> +\t\t\tsprintf(display, \"= %-*s %-*s -> %s\", TRANSPORT_SUMMARY_WIDTH,\n>  \t\t\t\t\"[up to date]\", REFCOL_WIDTH, remote,\n>  \t\t\t\tpretty_ref);\n\nIf you are refactoring, can all of these fetch lines just call\nprint_ref_status, which handles the summary width stuff itself? The push\nand fetch formats are meant to be quite similar.\n\n-Peff\n"},{"id":"134768","messageId":"7vhbphm0rn.fsf@alter.siamese.dyndns.org","threadId":"22668","inReplyTo":"1266276411-5796-4-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH v2 3/4] Refactoring: move duplicated code from builtin-pack-objects.c and fast-import.c to object.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T19:35:56Z","receivedAt":"2010-02-16T19:35:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Lukashov <michael.lukashov@gmail.com> writes:\n\n> The following functions are duplicated:\n>\n>   encode_header\n\nwhat are the other duplicated ones ;-)?\n\n> Signed-off-by: Michael Lukashov <michael.lukashov@gmail.com>\n> ---\n\nTwo comments:\n\n - encode_header() was a perfectly good name for a static function in\n   these two contexts, but when lifted into public namespace, it is not\n   clear enough anymore.  It is not clear \"header\" in what context you are\n   talking about.  At least it should be encode_in_pack_object_header();\n\n - Look at what are in object.[ch]; they are all about \"object\" layer,\n   that sits one level higher in the abstraction on top of the raw object\n   data layer (e.g. read_sha1_file() and friends).  This function belongs\n   to a layer that is even lower level than the raw object data (i.e. one\n   particular implementation of the raw object data representations among\n   others).\n\n   It looks very out of place.  I would say that cache.h and sha1_file.c\n   would probably be a better place, if nobody else finds a better\n   alternative.\n\nOther than that, I agree with the patch, including its choice of types\ninvolved.\n"},{"id":"134780","messageId":"7vd405m0hm.fsf@alter.siamese.dyndns.org","threadId":"22668","inReplyTo":"1266276411-5796-5-git-send-email-michael.lukashov@gmail.com","subject":"Re: [PATCH v2 4/4] Refactoring: remove duplicated code from builtin-checkout.c and merge-recursive.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-02-16T19:41:57Z","receivedAt":"2010-02-16T19:41:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Lukashov <michael.lukashov@gmail.com> writes:\n\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index 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;\n\nIsn't a much better home for this function next to read_mmfile() in\nxdiff-interface.c?\n\nPerhaps it would make sense to morph it into something like this\n\n\tint read_mmblob(mmfile_t *ptr, const unsigned char *sha1);\n\nfor consistency.\n"}]}