{"thread":{"id":"18048","subject":"[PATCH 07a/21] remote: make copy_ref() perform a deep copy","startedAt":"2009-02-27T19:10:03Z","lastAt":"2009-02-28T06:33:22Z","messageCount":5,"participants":["Jay Soffian","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"106459","messageId":"cover.1235759631.git.jaysoffian@gmail.com","threadId":"18048","inReplyTo":"20090227114308.GA20776@coredump.intra.peff.net","subject":"[PATCH 0/3] git remote: set-head and new show output (UPDATED)","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-27T19:10:03Z","receivedAt":"2009-02-27T19:10:03Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Jeff, Junio,\n\nI didn't want to resend the whole series (gmane 111394), so what I did was\ncreate a new patch which comes after patch 7 in that series. Call it patch 7a.\nThe rest of that series is then identical (modulo line number offsets in\npatches touching remote.c) except for patches 8 and 19. Patches 7a, and the\nnew 8 and 19 follow this email.\n\nYou should be able to do this:\n\n$ git checkout js/remote-improvements\n$ git format-patch master\n$ git reset --hard $(git merge-base master HEAD)\n\nCopy new \"[PATCH 07a/21] remote: make copy_ref() perform a deep copy\"\nto 0007a-remote-make-copy_ref-perform-a-deep-copy.patch\n\nCopy new \"[PATCH 08/21] remote: let guess_remote_head() optionally return all matches\"\nto 0008-remote-let-guess_remote_head-optionally-return-al.patch\n\nCopy new \"[PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available\"\nto 0019-remote-make-guess_remote_head-use-exact-HEAD-look.patch\n\n$ git am -3 *.patch\n\nFor those playing along at home, js/remote-improvements is currently\ncommits 661763a..5e4c90a.\n\nWhich should give you a new js/remote-improvements that looks like this:\n\n  1  test scripts: refactor start_httpd helper\n  2  add basic http clone/fetch tests\n  3  refactor find_ref_by_name() to accept const list\n  4  move duplicated get_local_heads() to remote.c\n  5  move duplicated ref_newer() to remote.c\n  6  move locate_head() to remote.c\n  7  remote: simplify guess_remote_head()\n  8  remote: make copy_ref() perform a deep copy\n  9  remote: let guess_remote_head() optionally return all matches\n 10  remote: make match_refs() copy src ref before assigning to peer_ref\n 11  remote: make match_refs() not short-circuit\n 12  string-list: new for_each_string_list() function\n 13  builtin-remote: refactor duplicated cleanup code\n 14  builtin-remote: remove unused code in get_ref_states\n 15  builtin-remote: rename variables and eliminate redundant function call\n 16  builtin-remote: make get_remote_ref_states() always populate states.tracked\n 17  builtin-remote: fix two inconsistencies in the output of \"show <remote>\"\n 18  builtin-remote: teach show to display remote HEAD\n 19  builtin-remote: add set-head subcommand\n 20  remote: make guess_remote_head() use exact HEAD lookup if it is available\n 21  builtin-remote: new show output style\n 22  builtin-remote: new show output style for push refspecs\n\nHere's the new diff stat:\n\n Documentation/git-remote.txt           |   28 ++-\n Makefile                               |    1 +\n builtin-clone.c                        |   41 +---\n builtin-remote.c                       |  561 ++++++++++++++++++++++++++------\n builtin-send-pack.c                    |   79 +-----\n cache.h                                |    2 +-\n contrib/completion/git-completion.bash |    2 +-\n http-push.c                            |   72 +----\n refs.c                                 |    4 +-\n remote.c                               |  145 ++++++++-\n remote.h                               |   12 +\n string-list.c                          |   10 +\n string-list.h                          |    5 +\n t/lib-httpd.sh                         |    9 +-\n t/t5505-remote.sh                      |  114 +++++--\n t/t5540-http-push.sh                   |    9 +-\n t/t5550-http-fetch.sh                  |   57 ++++\n 17 files changed, 821 insertions(+), 330 deletions(-)\n create mode 100755 t/t5550-http-fetch.sh\n\nAnd here's the inter-diff:\n\ndiff --git a/remote.c b/remote.c\nindex 1c09cf0..9b8522d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -779,10 +779,18 @@ struct ref *alloc_ref(const char *name)\n \n static struct ref *copy_ref(const struct ref *ref)\n {\n-\tstruct ref *ret = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);\n-\tmemcpy(ret, ref, sizeof(struct ref) + strlen(ref->name) + 1);\n-\tret->next = NULL;\n-\treturn ret;\n+\tstruct ref *cpy;\n+\tsize_t len;\n+\tif (!ref)\n+\t\treturn NULL;\n+\tlen = strlen(ref->name);\n+\tcpy = xmalloc(sizeof(struct ref) + len + 1);\n+\tmemcpy(cpy, ref, sizeof(struct ref) + len + 1);\n+\tcpy->next = NULL;\n+\tcpy->symref = ref->symref ? xstrdup(ref->symref) : NULL;\n+\tcpy->remote_status = ref->remote_status ? xstrdup(ref->remote_status) : NULL;\n+\tcpy->peer_ref = copy_ref(ref->peer_ref);\n+\treturn cpy;\n }\n \n struct ref *copy_ref_list(const struct ref *ref)\n@@ -801,6 +809,7 @@ static void free_ref(struct ref *ref)\n {\n \tif (!ref)\n \t\treturn;\n+\tfree_ref(ref->peer_ref);\n \tfree(ref->remote_status);\n \tfree(ref->symref);\n \tfree(ref);\n@@ -811,7 +820,6 @@ void free_refs(struct ref *ref)\n \tstruct ref *next;\n \twhile (ref) {\n \t\tnext = ref->next;\n-\t\tfree(ref->peer_ref);\n \t\tfree_ref(ref);\n \t\tref = next;\n \t}\n@@ -1457,13 +1465,6 @@ struct ref *get_local_heads(void)\n \treturn local_refs;\n }\n \n-struct ref *copy_ref_with_peer(const struct ref *src)\n-{\n-\tstruct ref *dst = copy_ref(src);\n-\tdst->peer_ref = copy_ref(src->peer_ref);\n-\treturn dst;\n-}\n-\n struct ref *guess_remote_head(const struct ref *head,\n \t\t\t      const struct ref *refs,\n \t\t\t      int all)\n@@ -1480,22 +1481,20 @@ struct ref *guess_remote_head(const struct ref *head,\n \t * where HEAD points; if that is the case, then\n \t * we don't have to guess.\n \t */\n-\tif (head->symref) {\n-\t\tr = find_ref_by_name(refs, head->symref);\n-\t\treturn r ? copy_ref_with_peer(r) : NULL;\n-\t}\n+\tif (head->symref)\n+\t\treturn copy_ref(find_ref_by_name(refs, head->symref));\n \n \t/* If refs/heads/master could be right, it is. */\n \tif (!all) {\n \t\tr = find_ref_by_name(refs, \"refs/heads/master\");\n \t\tif (r && !hashcmp(r->old_sha1, head->old_sha1))\n-\t\t\treturn copy_ref_with_peer(r);\n+\t\t\treturn copy_ref(r);\n \t}\n \n \t/* Look for another ref that points there */\n \tfor (r = refs; r; r = r->next) {\n \t\tif (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {\n-\t\t\t*tail = copy_ref_with_peer(r);\n+\t\t\t*tail = copy_ref(r);\n \t\t\ttail = &((*tail)->next);\n \t\t\tif (!all)\n \t\t\t\tbreak;\n"},{"id":"106458","messageId":"45e13cde37f6da8e6c379b5deea81b00148b4355.1235759631.git.jaysoffian@gmail.com","threadId":"18048","inReplyTo":"cover.1235759631.git.jaysoffian@gmail.com","subject":"[PATCH 07a/21] remote: make copy_ref() perform a deep copy","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-27T19:10:04Z","receivedAt":"2009-02-27T19:10:04Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"To ensure that copied refs can always be freed w/o causing a\ndouble-free, make copy_ref() perform a deep copy.\n\nAlso have copy_ref() return NULL if asked to copy NULL to simplify\nthings for the caller.\n\nBackground: currently copy_ref() performs a shallow copy. This is fine\nfor current callers who never free the result and/or only copy refs\nwhich contain NULL pointers. But copy_ref() is about to gain a new\ncaller (guess_remote_head()) which copies refs where peer_ref is not\nNULL and the caller of guess_remote_head() will want to free the result.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nThis is a new patch, which should come after 07/21 in the original\nseries - http://article.gmane.org/gmane.comp.version-control.git/111394\n\n remote.c |   18 +++++++++++++-----\n 1 files changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex aed760e..22203ea 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -779,10 +779,18 @@ struct ref *alloc_ref(const char *name)\n \n static struct ref *copy_ref(const struct ref *ref)\n {\n-\tstruct ref *ret = xmalloc(sizeof(struct ref) + strlen(ref->name) + 1);\n-\tmemcpy(ret, ref, sizeof(struct ref) + strlen(ref->name) + 1);\n-\tret->next = NULL;\n-\treturn ret;\n+\tstruct ref *cpy;\n+\tsize_t len;\n+\tif (!ref)\n+\t\treturn NULL;\n+\tlen = strlen(ref->name);\n+\tcpy = xmalloc(sizeof(struct ref) + len + 1);\n+\tmemcpy(cpy, ref, sizeof(struct ref) + len + 1);\n+\tcpy->next = NULL;\n+\tcpy->symref = ref->symref ? xstrdup(ref->symref) : NULL;\n+\tcpy->remote_status = ref->remote_status ? xstrdup(ref->remote_status) : NULL;\n+\tcpy->peer_ref = copy_ref(ref->peer_ref);\n+\treturn cpy;\n }\n \n struct ref *copy_ref_list(const struct ref *ref)\n@@ -801,6 +809,7 @@ static void free_ref(struct ref *ref)\n {\n \tif (!ref)\n \t\treturn;\n+\tfree_ref(ref->peer_ref);\n \tfree(ref->remote_status);\n \tfree(ref->symref);\n \tfree(ref);\n@@ -811,7 +820,6 @@ void free_refs(struct ref *ref)\n \tstruct ref *next;\n \twhile (ref) {\n \t\tnext = ref->next;\n-\t\tfree(ref->peer_ref);\n \t\tfree_ref(ref);\n \t\tref = next;\n \t}\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106460","messageId":"9d146489b82f3f8108903dcb9acbe569a4b690a1.1235759631.git.jaysoffian@gmail.com","threadId":"18048","inReplyTo":"45e13cde37f6da8e6c379b5deea81b00148b4355.1235759631.git.jaysoffian@gmail.com","subject":"[PATCH 08/21] remote: let guess_remote_head() optionally return all matches","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-27T19:10:05Z","receivedAt":"2009-02-27T19:10:05Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Determining HEAD is ambiguous since it is done by comparing SHA1s.\n\nIn the case of multiple matches we return refs/heads/master if it\nmatches, else we return the first match we encounter. builtin-remote\nneeds all matches returned to it, so add a flag for it to request such.\n\nTo be simple and consistent, the return value is now a copy (including\npeer_ref) of the matching refs.\n\nOriginally contributed by Jeff King along with the prior commit as a\nsingle patch.\n\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nThis patch replaces 08/21 in the original series\nseries - http://article.gmane.org/gmane.comp.version-control.git/111394\n\n builtin-clone.c |    2 +-\n remote.c        |   29 +++++++++++++++++++----------\n remote.h        |   14 ++++++++------\n 3 files changed, 28 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin-clone.c b/builtin-clone.c\nindex f9ce4fb..3146ca8 100644\n--- a/builtin-clone.c\n+++ b/builtin-clone.c\n@@ -510,7 +510,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tmapped_refs = write_remote_refs(refs, &refspec, reflog_msg.buf);\n \n \t\tremote_head = find_ref_by_name(refs, \"HEAD\");\n-\t\thead_points_at = guess_remote_head(remote_head, mapped_refs);\n+\t\thead_points_at = guess_remote_head(remote_head, mapped_refs, 0);\n \t}\n \telse {\n \t\twarning(\"You appear to have cloned an empty repository.\");\ndiff --git a/remote.c b/remote.c\nindex 22203ea..304e967 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1460,24 +1460,33 @@ struct ref *get_local_heads(void)\n \treturn local_refs;\n }\n \n-const struct ref *guess_remote_head(const struct ref *head,\n-\t\t\t\t    const struct ref *refs)\n+struct ref *guess_remote_head(const struct ref *head,\n+\t\t\t      const struct ref *refs,\n+\t\t\t      int all)\n {\n \tconst struct ref *r;\n+\tstruct ref *list = NULL;\n+\tstruct ref **tail = &list;\n \n \tif (!head)\n \t\treturn NULL;\n \n \t/* If refs/heads/master could be right, it is. */\n-\tr = find_ref_by_name(refs, \"refs/heads/master\");\n-\tif (r && !hashcmp(r->old_sha1, head->old_sha1))\n-\t\treturn r;\n+\tif (!all) {\n+\t\tr = find_ref_by_name(refs, \"refs/heads/master\");\n+\t\tif (r && !hashcmp(r->old_sha1, head->old_sha1))\n+\t\t\treturn copy_ref(r);\n+\t}\n \n \t/* Look for another ref that points there */\n-\tfor (r = refs; r; r = r->next)\n-\t\tif (r != head && !hashcmp(r->old_sha1, head->old_sha1))\n-\t\t\treturn r;\n+\tfor (r = refs; r; r = r->next) {\n+\t\tif (r != head && !hashcmp(r->old_sha1, head->old_sha1)) {\n+\t\t\t*tail = copy_ref(r);\n+\t\t\ttail = &((*tail)->next);\n+\t\t\tif (!all)\n+\t\t\t\tbreak;\n+\t\t}\n+\t}\n \n-\t/* Nothing is the same */\n-\treturn NULL;\n+\treturn list;\n }\ndiff --git a/remote.h b/remote.h\nindex db49ce0..de3d21b 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -139,12 +139,14 @@ int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs);\n int format_tracking_info(struct branch *branch, struct strbuf *sb);\n \n struct ref *get_local_heads(void);\n-\n /*\n- * Look for a ref in refs whose SHA1 matches head, first checking if\n- * refs/heads/master matches. Return NULL if nothing matches or if head\n- * is NULL.\n+ * Find refs from a list which are likely to be pointed to by the given HEAD\n+ * ref. If 'all' is false, returns the most likely ref; otherwise, returns a\n+ * list of all candidate refs. If no match is found (or 'head' is NULL),\n+ * returns NULL. All returns are newly allocated and should be freed.\n  */\n-const struct ref *guess_remote_head(const struct ref *head,\n-\t\t\t\t    const struct ref *refs);\n+struct ref *guess_remote_head(const struct ref *head,\n+\t\t\t      const struct ref *refs,\n+\t\t\t      int all);\n+\n #endif\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106461","messageId":"710d1a0d6059e6dee31732c2762aed7e909daad7.1235759631.git.jaysoffian@gmail.com","threadId":"18048","inReplyTo":"9d146489b82f3f8108903dcb9acbe569a4b690a1.1235759631.git.jaysoffian@gmail.com","subject":"[PATCH 19/21] remote: make guess_remote_head() use exact HEAD lookup if it is available","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-02-27T19:10:06Z","receivedAt":"2009-02-27T19:10:06Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"From: Jeff King <peff@peff.net>\n\nOur usual method for determining the ref pointed to by HEAD\nis to compare HEAD's sha1 to the sha1 of all refs, trying to\nfind a unique match.\n\nHowever, some transports actually get to look at HEAD\ndirectly; we should make use of that information when it is\navailable.  Currently, only http remotes support this\nfeature.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Jay Soffian <jaysoffian@gmail.com>\n---\nThis patch replaces 19/21 in the original series\nseries - http://article.gmane.org/gmane.comp.version-control.git/111394\n\n remote.c              |    8 ++++++++\n t/t5550-http-fetch.sh |   11 +++++++++++\n 2 files changed, 19 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 2123005..9b8522d 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1476,6 +1476,14 @@ struct ref *guess_remote_head(const struct ref *head,\n \tif (!head)\n \t\treturn NULL;\n \n+\t/*\n+\t * Some transports support directly peeking at\n+\t * where HEAD points; if that is the case, then\n+\t * we don't have to guess.\n+\t */\n+\tif (head->symref)\n+\t\treturn copy_ref(find_ref_by_name(refs, head->symref));\n+\n \t/* If refs/heads/master could be right, it is. */\n \tif (!all) {\n \t\tr = find_ref_by_name(refs, \"refs/heads/master\");\ndiff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh\nindex b6e6ec9..05b1b62 100755\n--- a/t/t5550-http-fetch.sh\n+++ b/t/t5550-http-fetch.sh\n@@ -42,5 +42,16 @@ test_expect_success 'fetch changes via http' '\n \ttest_cmp file clone/file\n '\n \n+test_expect_success 'http remote detects correct HEAD' '\n+\tgit push public master:other &&\n+\t(cd clone &&\n+\t git remote set-head origin -d &&\n+\t git remote set-head origin -a &&\n+\t git symbolic-ref refs/remotes/origin/HEAD > output &&\n+\t echo refs/remotes/origin/master > expect &&\n+\t test_cmp expect output\n+\t)\n+'\n+\n stop_httpd\n test_done\n-- \n1.6.2.rc1.309.g5f417\n"},{"id":"106513","messageId":"20090228063322.GA6775@coredump.intra.peff.net","threadId":"18048","inReplyTo":"cover.1235759631.git.jaysoffian@gmail.com","subject":"Re: [PATCH 0/3] git remote: set-head and new show output (UPDATED)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-28T06:33:22Z","receivedAt":"2009-02-28T06:33:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 27, 2009 at 02:10:03PM -0500, Jay Soffian wrote:\n\n> I didn't want to resend the whole series (gmane 111394), so what I did was\n> create a new patch which comes after patch 7 in that series. Call it patch 7a.\n> The rest of that series is then identical (modulo line number offsets in\n> patches touching remote.c) except for patches 8 and 19. Patches 7a, and the\n> new 8 and 19 follow this email.\n\nThese look good to me.\n\n-Peff\n"}]}