{"thread":{"id":"13469","subject":"[PATCH] Optimize common pattern of alloc_ref from string","startedAt":"2008-05-10T23:26:58Z","lastAt":"2008-05-11T22:02:45Z","messageCount":8,"participants":["kkowalczyk@gmail.com","Jeff King","Krzysztof Kowalczyk","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"76557","messageId":"1210462018-47060-1-git-send-email-kkowalczyk@gmail.com","threadId":"13469","inReplyTo":null,"subject":"[PATCH] Optimize common pattern of alloc_ref from string","fromName":"","fromEmail":"kkowalczyk@gmail.com","sentAt":"2008-05-10T23:26:58Z","receivedAt":"2008-05-10T23:26:58Z","isPatch":true,"sender":{"key":"kkowalczyk@gmail.com","avatar":"https://gravatar.com/avatar/e959d54bb779ff1db95338a1ca1955bd832c8083ac9e92b344028ab2f10e4b4c?d=mp&s=160"},"body":"From: Krzysztof Kowalczyk <kkowalczyk@gmail.com>\n\nAs a byproduct, fixes one place where string wasn't properly terminated.\nSigned-off-by: Krzysztof Kowalczyk <kkowalczyk@gmail.com>\n---\n builtin-fetch.c |    6 ++----\n http-push.c     |    6 ++----\n remote.c        |   28 +++++++++++++++-------------\n remote.h        |    2 ++\n transport.c     |    6 ++----\n walker.c        |    3 +--\n 6 files changed, 24 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex e56617e..f6584ec 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -508,10 +508,8 @@ static void find_non_local_tags(struct transport *transport,\n \t\t     will_fetch(head, ref->old_sha1))) {\n \t\t\tpath_list_insert(ref_name, &new_refs);\n \n-\t\t\trm = alloc_ref(strlen(ref_name) + 1);\n-\t\t\tstrcpy(rm->name, ref_name);\n-\t\t\trm->peer_ref = alloc_ref(strlen(ref_name) + 1);\n-\t\t\tstrcpy(rm->peer_ref->name, ref_name);\n+\t\t\trm = alloc_ref_from_str(ref_name);\n+\t\t\trm->peer_ref = alloc_ref_from_str(ref_name);\n \t\t\thashcpy(rm->old_sha1, ref_sha1);\n \n \t\t\t**tail = rm;\ndiff --git a/http-push.c b/http-push.c\nindex 939a764..42727c8 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1761,8 +1761,7 @@ static void one_remote_ref(char *refname)\n \tstruct ref *ref;\n \tstruct object *obj;\n \n-\tref = alloc_ref(strlen(refname) + 1);\n-\tstrcpy(ref->name, refname);\n+\tref = alloc_ref_from_str(refname);\n \n \tif (http_fetch_ref(remote->url, ref) != 0) {\n \t\tfprintf(stderr,\n@@ -1894,8 +1893,7 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)\n \tchar *ref_info;\n \tstruct ref *ref;\n \n-\tref = alloc_ref(strlen(ls->dentry_name) + 1);\n-\tstrcpy(ref->name, ls->dentry_name);\n+\tref = alloc_ref_from_str(ls->dentry_name);\n \n \tif (http_fetch_ref(remote->url, ref) != 0) {\n \t\tfprintf(stderr,\ndiff --git a/remote.c b/remote.c\nindex 6b480cb..4d3e0c7 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -691,6 +691,18 @@ struct ref *alloc_ref(unsigned namelen)\n \treturn ret;\n }\n \n+struct ref *alloc_ref_from_str(const char* str)\n+{\n+\tstruct ref *ret;\n+\tunsigned len = strlen(str) + 1;\n+\tchar *tmp = xmalloc(sizeof(struct ref) + len);\n+\tret = (struct ref*)tmp;\n+\tmemset(tmp, 0, sizeof(struct ref));\n+\ttmp += sizeof(struct ref);\n+\tmemcpy(tmp, str, len);\n+\treturn ret;\n+}\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@@ -797,7 +809,6 @@ static struct ref *try_explicit_object_name(const char *name)\n {\n \tunsigned char sha1[20];\n \tstruct ref *ref;\n-\tint len;\n \n \tif (!*name) {\n \t\tref = alloc_ref(20);\n@@ -807,21 +818,14 @@ static struct ref *try_explicit_object_name(const char *name)\n \t}\n \tif (get_sha1(name, sha1))\n \t\treturn NULL;\n-\tlen = strlen(name) + 1;\n-\tref = alloc_ref(len);\n-\tmemcpy(ref->name, name, len);\n+\tref = alloc_ref_from_str(name);\n \thashcpy(ref->new_sha1, sha1);\n \treturn ref;\n }\n \n static struct ref *make_linked_ref(const char *name, struct ref ***tail)\n {\n-\tstruct ref *ret;\n-\tsize_t len;\n-\n-\tlen = strlen(name) + 1;\n-\tret = alloc_ref(len);\n-\tmemcpy(ret->name, name, len);\n+\tstruct ref *ret = alloc_ref_from_str(name);\n \ttail_link_ref(ret, tail);\n \treturn ret;\n }\n@@ -1125,9 +1129,7 @@ static struct ref *get_local_ref(const char *name)\n \t\treturn NULL;\n \n \tif (!prefixcmp(name, \"refs/\")) {\n-\t\tret = alloc_ref(strlen(name) + 1);\n-\t\tstrcpy(ret->name, name);\n-\t\treturn ret;\n+\t\treturn alloc_ref_from_str(name);\n \t}\n \n \tif (!prefixcmp(name, \"heads/\") ||\ndiff --git a/remote.h b/remote.h\nindex 75d006b..2ee83a3 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -54,6 +54,8 @@ struct refspec {\n \n struct ref *alloc_ref(unsigned namelen);\n \n+struct ref *alloc_ref_from_str(const char* str);\n+\n struct ref *copy_ref_list(const struct ref *ref);\n \n int check_ref_type(const struct ref *ref, int flags);\ndiff --git a/transport.c b/transport.c\nindex b012a28..1bc16f2 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -504,8 +504,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \n \tstrbuf_release(&buffer);\n \n-\tref = alloc_ref(strlen(\"HEAD\") + 1);\n-\tstrcpy(ref->name, \"HEAD\");\n+\tref = alloc_ref_from_str(\"HEAD\");\n \tif (!walker->fetch_ref(walker, ref) &&\n \t    !resolve_remote_symref(ref, refs)) {\n \t\tref->next = refs;\n@@ -546,9 +545,8 @@ static struct ref *get_refs_from_bundle(struct transport *transport)\n \t\tdie (\"Could not read bundle '%s'.\", transport->url);\n \tfor (i = 0; i < data->header.references.nr; i++) {\n \t\tstruct ref_list_entry *e = data->header.references.list + i;\n-\t\tstruct ref *ref = alloc_ref(strlen(e->name) + 1);\n+\t\tstruct ref *ref = alloc_ref_from_str(e->name);\n \t\thashcpy(ref->old_sha1, e->sha1);\n-\t\tstrcpy(ref->name, e->name);\n \t\tref->next = result;\n \t\tresult = ref;\n \t}\ndiff --git a/walker.c b/walker.c\nindex fa96a7c..31de6c1 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -190,8 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *\n \tif (!get_sha1_hex(target, sha1))\n \t\treturn 0;\n \tif (!check_ref_format(target)) {\n-\t\tstruct ref *ref = alloc_ref(strlen(target));\n-\t\tstrcpy(ref->name, target);\n+\t\tstruct ref *ref = alloc_ref_from_str(target);\n \t\tif (!walker->fetch_ref(walker, ref)) {\n \t\t\thashcpy(sha1, ref->old_sha1);\n \t\t\tfree(ref);\n-- \n1.5.4.5\n"},{"id":"76559","messageId":"20080510233918.GA315@sigill.intra.peff.net","threadId":"13469","inReplyTo":"1210462018-47060-1-git-send-email-kkowalczyk@gmail.com","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-10T23:39:19Z","receivedAt":"2008-05-10T23:39:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 10, 2008 at 04:26:58PM -0700, kkowalczyk@gmail.com wrote:\n\n> As a byproduct, fixes one place where string wasn't properly terminated.\n\nGreat. Does this fix a user-visible bug? It would be nice to mention in\nthe commit log _which_ place (though after reading the patch carefully,\nit looks like the one interpret_target) so that people looking at the\ncommit later can understand exactly what was fixed.\n\n> -\tref = alloc_ref(strlen(refname) + 1);\n> -\tstrcpy(ref->name, refname);\n> +\tref = alloc_ref_from_str(refname);\n\nSo this turns a 2-line construct into a 1-line construct...\n\n> +struct ref *alloc_ref_from_str(const char* str)\n> +{\n> +\tstruct ref *ret;\n> +\tunsigned len = strlen(str) + 1;\n> +\tchar *tmp = xmalloc(sizeof(struct ref) + len);\n> +\tret = (struct ref*)tmp;\n> +\tmemset(tmp, 0, sizeof(struct ref));\n> +\ttmp += sizeof(struct ref);\n> +\tmemcpy(tmp, str, len);\n> +\treturn ret;\n> +}\n\nBut why do we need an 8-line function to do it?\n\nThe only difference I can see over\n\n  struct ref *alloc_ref_from_str(const char *str)\n  {\n    unsigned len = strlen(str) + 1;\n    struct ref *ret = alloc_ref(len);\n    memcpy(ret->name, str, len);\n    return ret;\n  }\n\nis that we avoid memsetting the name portion of the struct to 0 before\ncopying to it. It seems like an unproven micro-optimization that makes\nit a bit harder to read.\n\n-Peff\n"},{"id":"76564","messageId":"7ce338ad0805101730n5b964a0em39d9fdcd9fc45f00@mail.gmail.com","threadId":"13469","inReplyTo":"20080510233918.GA315@sigill.intra.peff.net","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Krzysztof Kowalczyk","fromEmail":"kkowalczyk@gmail.com","sentAt":"2008-05-11T00:30:56Z","receivedAt":"2008-05-11T00:30:56Z","isPatch":true,"sender":{"key":"kkowalczyk@gmail.com","avatar":"https://gravatar.com/avatar/e959d54bb779ff1db95338a1ca1955bd832c8083ac9e92b344028ab2f10e4b4c?d=mp&s=160"},"body":"On Sat, May 10, 2008 at 4:39 PM, Jeff King <peff@peff.net> wrote:\n> On Sat, May 10, 2008 at 04:26:58PM -0700, kkowalczyk@gmail.com wrote:\n>\n>> As a byproduct, fixes one place where string wasn't properly terminated.\n>\n> Great. Does this fix a user-visible bug? It would be nice to mention in\n> the commit log _which_ place (though after reading the patch carefully,\n> it looks like the one interpret_target) so that people looking at the\n> commit later can understand exactly what was fixed.\n\nIt was a subtle memory corruption that wouldn't cause problems in\n99.99% cases, but valgrind would probably catch it. And yes, it's the\ninterp_target().\n\n>> -     ref = alloc_ref(strlen(refname) + 1);\n>> -     strcpy(ref->name, refname);\n>> +     ref = alloc_ref_from_str(refname);\n>\n> So this turns a 2-line construct into a 1-line construct...\n\nAnd avoids future prossible mistakes with not terminating the string,\nlike the one just commited.\n\n>> +struct ref *alloc_ref_from_str(const char* str)\n>> +{\n>> +     struct ref *ret;\n>> +     unsigned len = strlen(str) + 1;\n>> +     char *tmp = xmalloc(sizeof(struct ref) + len);\n>> +     ret = (struct ref*)tmp;\n>> +     memset(tmp, 0, sizeof(struct ref));\n>> +     tmp += sizeof(struct ref);\n>> +     memcpy(tmp, str, len);\n>> +     return ret;\n>> +}\n>\n> But why do we need an 8-line function to do it?\n>\n> The only difference I can see over\n>\n>  struct ref *alloc_ref_from_str(const char *str)\n>  {\n>    unsigned len = strlen(str) + 1;\n>    struct ref *ret = alloc_ref(len);\n>    memcpy(ret->name, str, len);\n>    return ret;\n>  }\n>\n> is that we avoid memsetting the name portion of the struct to 0 before\n> copying to it. It seems like an unproven micro-optimization that makes\n> it a bit harder to read.\n\nYou're absolutely right - it's a micro-optimization and your version\nmight be preferred for clarity. This is the first time I submit a\npatch to git so I don't have a good feel for what kind of treadoffs\npeople find acceptable.\n\nI should also mention that\nstatic struct ref *try_explicit_object_name(const char *name)\n{\n\tunsigned char sha1[20];\n\tstruct ref *ref;\n\n\tif (!*name) {\n\t\tref = alloc_ref(20);\n\t\tstrcpy(ref->name, \"(delete)\");\n\t\thashclr(ref->new_sha1);\n\t\treturn ref;\n\t}\n...\n\ncould also be replaced with alloc_ref_str() - I just wasn't 100% sure\nif overallocating 10 bytes (20 - strlen(\"(delete)\")) was just sloppy\ncode or does other code relies on that (which is unlikely and if true\nthen it wouldn't be good).\n\nRegards,\n\n-- kjk\n"},{"id":"76590","messageId":"20080511080709.GA6971@sigill.intra.peff.net","threadId":"13469","inReplyTo":"7ce338ad0805101730n5b964a0em39d9fdcd9fc45f00@mail.gmail.com","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-05-11T08:07:09Z","receivedAt":"2008-05-11T08:07:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, May 10, 2008 at 05:30:56PM -0700, Krzysztof Kowalczyk wrote:\n\n> >> -     ref = alloc_ref(strlen(refname) + 1);\n> >> -     strcpy(ref->name, refname);\n> >> +     ref = alloc_ref_from_str(refname);\n> >\n> > So this turns a 2-line construct into a 1-line construct...\n> \n> And avoids future prossible mistakes with not terminating the string,\n> like the one just commited.\n\nYes. Please don't interpret my comment as \"this change isn't worth it\";\nI meant it as \"yes, it is good to be simplifying and making this part\nless error prone; I just want to nitpick your exact implementation\".\n\n> You're absolutely right - it's a micro-optimization and your version\n> might be preferred for clarity. This is the first time I submit a\n> patch to git so I don't have a good feel for what kind of treadoffs\n> people find acceptable.\n\nOK. Ultimately it is up to Junio. I think your version is a bit complex,\nbut at the very least it contains that complexity neatly in one function.\n\nActually, the version I posted actually has an optimization, too (it\nremembers the strlen calculation to reuse it). I think the simplest\nwould just be:\n\n  struct ref *alloc_ref_from_str(cons char *str)\n  {\n    struct ref *ret = alloc_ref(strlen(str) + 1);\n    strcpy(ret->name, str);\n    return ret;\n  }\n\nBut really my main worry is that now we have _two_ functions which\nallocate refs, so if \"struct ref\" ever grows a new field that needs\ninitializing, it has to go in two places (whereas if alloc_ref_from_str\ncalls alloc_ref, it works automatically).\n\n> I should also mention that\n> static struct ref *try_explicit_object_name(const char *name)\n> {\n> \tunsigned char sha1[20];\n> \tstruct ref *ref;\n> \n> \tif (!*name) {\n> \t\tref = alloc_ref(20);\n> \t\tstrcpy(ref->name, \"(delete)\");\n> \t\thashclr(ref->new_sha1);\n> \t\treturn ref;\n> \t}\n> ...\n> \n> could also be replaced with alloc_ref_str() - I just wasn't 100% sure\n> if overallocating 10 bytes (20 - strlen(\"(delete)\")) was just sloppy\n> code or does other code relies on that (which is unlikely and if true\n> then it wouldn't be good).\n\nIt looks like just slop, and should probably be fixed at the same time.\nThere are also quite a number of:\n\n  ret = alloc_ref(strlen(name) + 6);\n  sprintf(ret->name, \"refs/%s\", name);\n\nwhich are error-prone and owuld be nice to fix. However, I don't think\nthere is an easy way short of making ref->name a strbuf. And now it\nseems we are getting into a lot of code churn for a relatively small\nbenefit.\n\n-Peff\n"},{"id":"76621","messageId":"alpine.DEB.1.00.0805111630590.30431@racer","threadId":"13469","inReplyTo":"1210462018-47060-1-git-send-email-kkowalczyk@gmail.com","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-11T15:32:19Z","receivedAt":"2008-05-11T15:32:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 10 May 2008, kkowalczyk@gmail.com wrote:\n\n> diff --git a/builtin-fetch.c b/builtin-fetch.c\n> index e56617e..f6584ec 100644\n> --- a/builtin-fetch.c\n> +++ b/builtin-fetch.c\n> @@ -508,10 +508,8 @@ static void find_non_local_tags(struct transport *transport,\n>  \t\t     will_fetch(head, ref->old_sha1))) {\n>  \t\t\tpath_list_insert(ref_name, &new_refs);\n>  \n> -\t\t\trm = alloc_ref(strlen(ref_name) + 1);\n> -\t\t\tstrcpy(rm->name, ref_name);\n> -\t\t\trm->peer_ref = alloc_ref(strlen(ref_name) + 1);\n> -\t\t\tstrcpy(rm->peer_ref->name, ref_name);\n> +\t\t\trm = alloc_ref_from_str(ref_name);\n> +\t\t\trm->peer_ref = alloc_ref_from_str(ref_name);\n\nThanks.  This was on my ever-growing TODO list (actually, fix alloc_ref() \nto take the string length instead of the size to be allocated).\n\nTogether with the comments from Peff I am very positive on this patch.\n\nThanks,\nDscho\n"},{"id":"76653","messageId":"7vskwo3ct5.fsf@gitster.siamese.dyndns.org","threadId":"13469","inReplyTo":"20080511080709.GA6971@sigill.intra.peff.net","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T18:39:34Z","receivedAt":"2008-05-11T18:39:34Z","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> But really my main worry is that now we have _two_ functions which\n> allocate refs, so if \"struct ref\" ever grows a new field that needs\n> initializing, it has to go in two places (whereas if alloc_ref_from_str\n> calls alloc_ref, it works automatically).\n\nThis is a very good point.  We really do not want a micro-optimization in\na way that hurts maintainability.\n\nKrzysztof's patch has the new function with the duplicated allocation\nimplementation between the base allocator function and the existing\ncopy_ref() function which also has yet another duplicated allocation\nimplementation.  When somebody needs to modify one, it is likely to be\nnoticed that these three go hand-in-hand, hopefully ;-).\n\nBy the way, why isn't alloc_ref() doing xcalloc(), I have to wonder...\n"},{"id":"76662","messageId":"7ce338ad0805111315q3745ea7fn4c38b557170e521f@mail.gmail.com","threadId":"13469","inReplyTo":"7vskwo3ct5.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Krzysztof Kowalczyk","fromEmail":"kkowalczyk@gmail.com","sentAt":"2008-05-11T20:15:52Z","receivedAt":"2008-05-11T20:15:52Z","isPatch":true,"sender":{"key":"kkowalczyk@gmail.com","avatar":"https://gravatar.com/avatar/e959d54bb779ff1db95338a1ca1955bd832c8083ac9e92b344028ab2f10e4b4c?d=mp&s=160"},"body":"On Sun, May 11, 2008 at 11:39 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jeff King <peff@peff.net> writes:\n>\n>> But really my main worry is that now we have _two_ functions which\n>> allocate refs, so if \"struct ref\" ever grows a new field that needs\n>> initializing, it has to go in two places (whereas if alloc_ref_from_str\n>> calls alloc_ref, it works automatically).\n>\n> This is a very good point.  We really do not want a micro-optimization in\n> a way that hurts maintainability.\n>\n> Krzysztof's patch has the new function with the duplicated allocation\n> implementation between the base allocator function and the existing\n> copy_ref() function which also has yet another duplicated allocation\n> implementation.  When somebody needs to modify one, it is likely to be\n> noticed that these three go hand-in-hand, hopefully ;-).\n>\n> By the way, why isn't alloc_ref() doing xcalloc(), I have to wonder...\n\nI've sent updated patch that incorporates the feedback. Changes wrt.\nto this patch:\n* simpler version of alloc_ref_from_str() per Jeff's suggestion\n* switches alloc_ref() to xcalloc() per Junio's suggestion\n* uses alloc_ref_from_str() in one more place\n* better (?) commit message\n\n-- kjk\n"},{"id":"76663","messageId":"7v8wyg33ei.fsf@gitster.siamese.dyndns.org","threadId":"13469","inReplyTo":"7ce338ad0805111315q3745ea7fn4c38b557170e521f@mail.gmail.com","subject":"Re: [PATCH] Optimize common pattern of alloc_ref from string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-11T22:02:45Z","receivedAt":"2008-05-11T22:02:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks.  I've already queued this instead, as it is not about\n\"Optimization\" but about refactoring.\n\nI am actually very tempted to relabel it as \"Fix underallocation in\ninterpret_target()\" and describe the refactoring just as a side-effect,\nbut the commit is already buried deep in my outgoing queue for \"master\",\nand I do not think it is worth delaying today's pushout for rewording the\ncommit log message again.\n\n-- >8 --\nFrom: Krzysztof Kowalczyk <kkowalczyk@gmail.com>\nDate: Sat, 10 May 2008 16:26:58 -0700\nSubject: [PATCH] alloc_ref_from_str(): factor out a common pattern of alloc_ref from string\n\nAlso fix an underallocation in walker.c::interpret_target().\n\nSigned-off-by: Krzysztof Kowalczyk <kkowalczyk@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-fetch.c |    6 ++----\n http-push.c     |    6 ++----\n remote.c        |   23 ++++++++++-------------\n remote.h        |    2 ++\n transport.c     |    6 ++----\n walker.c        |    3 +--\n 6 files changed, 19 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-fetch.c b/builtin-fetch.c\nindex e56617e..f6584ec 100644\n--- a/builtin-fetch.c\n+++ b/builtin-fetch.c\n@@ -508,10 +508,8 @@ static void find_non_local_tags(struct transport *transport,\n \t\t     will_fetch(head, ref->old_sha1))) {\n \t\t\tpath_list_insert(ref_name, &new_refs);\n \n-\t\t\trm = alloc_ref(strlen(ref_name) + 1);\n-\t\t\tstrcpy(rm->name, ref_name);\n-\t\t\trm->peer_ref = alloc_ref(strlen(ref_name) + 1);\n-\t\t\tstrcpy(rm->peer_ref->name, ref_name);\n+\t\t\trm = alloc_ref_from_str(ref_name);\n+\t\t\trm->peer_ref = alloc_ref_from_str(ref_name);\n \t\t\thashcpy(rm->old_sha1, ref_sha1);\n \n \t\t\t**tail = rm;\ndiff --git a/http-push.c b/http-push.c\nindex 939a764..42727c8 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -1761,8 +1761,7 @@ static void one_remote_ref(char *refname)\n \tstruct ref *ref;\n \tstruct object *obj;\n \n-\tref = alloc_ref(strlen(refname) + 1);\n-\tstrcpy(ref->name, refname);\n+\tref = alloc_ref_from_str(refname);\n \n \tif (http_fetch_ref(remote->url, ref) != 0) {\n \t\tfprintf(stderr,\n@@ -1894,8 +1893,7 @@ static void add_remote_info_ref(struct remote_ls_ctx *ls)\n \tchar *ref_info;\n \tstruct ref *ref;\n \n-\tref = alloc_ref(strlen(ls->dentry_name) + 1);\n-\tstrcpy(ref->name, ls->dentry_name);\n+\tref = alloc_ref_from_str(ls->dentry_name);\n \n \tif (http_fetch_ref(remote->url, ref) != 0) {\n \t\tfprintf(stderr,\ndiff --git a/remote.c b/remote.c\nindex 6b480cb..91cbb72 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -691,6 +691,13 @@ struct ref *alloc_ref(unsigned namelen)\n \treturn ret;\n }\n \n+struct ref *alloc_ref_from_str(const char* str)\n+{\n+\tstruct ref *ret = alloc_ref(strlen(str) + 1);\n+\tstrcpy(ret->name, str);\n+\treturn ret;\n+}\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@@ -797,7 +804,6 @@ static struct ref *try_explicit_object_name(const char *name)\n {\n \tunsigned char sha1[20];\n \tstruct ref *ref;\n-\tint len;\n \n \tif (!*name) {\n \t\tref = alloc_ref(20);\n@@ -807,21 +813,14 @@ static struct ref *try_explicit_object_name(const char *name)\n \t}\n \tif (get_sha1(name, sha1))\n \t\treturn NULL;\n-\tlen = strlen(name) + 1;\n-\tref = alloc_ref(len);\n-\tmemcpy(ref->name, name, len);\n+\tref = alloc_ref_from_str(name);\n \thashcpy(ref->new_sha1, sha1);\n \treturn ref;\n }\n \n static struct ref *make_linked_ref(const char *name, struct ref ***tail)\n {\n-\tstruct ref *ret;\n-\tsize_t len;\n-\n-\tlen = strlen(name) + 1;\n-\tret = alloc_ref(len);\n-\tmemcpy(ret->name, name, len);\n+\tstruct ref *ret = alloc_ref_from_str(name);\n \ttail_link_ref(ret, tail);\n \treturn ret;\n }\n@@ -1125,9 +1124,7 @@ static struct ref *get_local_ref(const char *name)\n \t\treturn NULL;\n \n \tif (!prefixcmp(name, \"refs/\")) {\n-\t\tret = alloc_ref(strlen(name) + 1);\n-\t\tstrcpy(ret->name, name);\n-\t\treturn ret;\n+\t\treturn alloc_ref_from_str(name);\n \t}\n \n \tif (!prefixcmp(name, \"heads/\") ||\ndiff --git a/remote.h b/remote.h\nindex 75d006b..2ee83a3 100644\n--- a/remote.h\n+++ b/remote.h\n@@ -54,6 +54,8 @@ struct refspec {\n \n struct ref *alloc_ref(unsigned namelen);\n \n+struct ref *alloc_ref_from_str(const char* str);\n+\n struct ref *copy_ref_list(const struct ref *ref);\n \n int check_ref_type(const struct ref *ref, int flags);\ndiff --git a/transport.c b/transport.c\nindex b012a28..1bc16f2 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -504,8 +504,7 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \n \tstrbuf_release(&buffer);\n \n-\tref = alloc_ref(strlen(\"HEAD\") + 1);\n-\tstrcpy(ref->name, \"HEAD\");\n+\tref = alloc_ref_from_str(\"HEAD\");\n \tif (!walker->fetch_ref(walker, ref) &&\n \t    !resolve_remote_symref(ref, refs)) {\n \t\tref->next = refs;\n@@ -546,9 +545,8 @@ static struct ref *get_refs_from_bundle(struct transport *transport)\n \t\tdie (\"Could not read bundle '%s'.\", transport->url);\n \tfor (i = 0; i < data->header.references.nr; i++) {\n \t\tstruct ref_list_entry *e = data->header.references.list + i;\n-\t\tstruct ref *ref = alloc_ref(strlen(e->name) + 1);\n+\t\tstruct ref *ref = alloc_ref_from_str(e->name);\n \t\thashcpy(ref->old_sha1, e->sha1);\n-\t\tstrcpy(ref->name, e->name);\n \t\tref->next = result;\n \t\tresult = ref;\n \t}\ndiff --git a/walker.c b/walker.c\nindex fa96a7c..31de6c1 100644\n--- a/walker.c\n+++ b/walker.c\n@@ -190,8 +190,7 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *\n \tif (!get_sha1_hex(target, sha1))\n \t\treturn 0;\n \tif (!check_ref_format(target)) {\n-\t\tstruct ref *ref = alloc_ref(strlen(target));\n-\t\tstrcpy(ref->name, target);\n+\t\tstruct ref *ref = alloc_ref_from_str(target);\n \t\tif (!walker->fetch_ref(walker, ref)) {\n \t\t\thashcpy(sha1, ref->old_sha1);\n \t\t\tfree(ref);\n-- \n1.5.5.1.294.g7b425\n"}]}