{"thread":{"id":"46816","subject":"[PATCH 1/3] refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional","startedAt":"2017-09-23T09:42:07Z","lastAt":"2017-10-01T08:29:18Z","messageCount":8,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"328723","messageId":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","threadId":"46816","inReplyTo":null,"subject":"[PATCH 1/3] refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-23T09:41:45Z","receivedAt":"2017-09-23T09:42:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Allow callers of refs_resolve_ref_unsafe() to pass NULL if they don't\nneed the resolved hash value.  We already allow the same for the flags\nparameter.  This new leniency is inherited by the various wrappers like\nresolve_ref_unsafe().\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n refs.c | 3 +++\n refs.h | 9 +++++----\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex c30f4c36be..36439fdb54 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1396,9 +1396,12 @@ const char *refs_resolve_ref_unsafe(struct ref_store *refs,\n \t\t\t\t    unsigned char *sha1, int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n+\tstruct object_id unused_oid;\n \tint unused_flags;\n \tint symref_count;\n \n+\tif (!sha1)\n+\t\tsha1 = unused_oid.hash;\n \tif (!flags)\n \t\tflags = &unused_flags;\n \ndiff --git a/refs.h b/refs.h\nindex 78a26400b6..996d4ea54d 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -10,10 +10,11 @@ struct worktree;\n /*\n  * Resolve a reference, recursively following symbolic refererences.\n  *\n- * Store the referred-to object's name in sha1 and return the name of\n- * the non-symbolic reference that ultimately pointed at it.  The\n- * return value, if not NULL, is a pointer into either a static buffer\n- * or the input ref.\n+ * Return the name of the non-symbolic reference that ultimately pointed\n+ * at the resolved object name.  The return value, if not NULL, is a\n+ * pointer into either a static buffer or the input ref.\n+ *\n+ * If sha1 is non-NULL, store the referred-to object's name in it.\n  *\n  * If the reference cannot be resolved to an object, the behavior\n  * depends on the RESOLVE_REF_READING flag:\n-- \n2.14.1\n\n"},{"id":"328724","messageId":"7ea7726d-0173-61f9-3056-939916e3f472@web.de","threadId":"46816","inReplyTo":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","subject":"[PATCH 2/3] refs: pass NULL to refs_resolve_ref_unsafe() if hash is not needed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-23T09:44:57Z","receivedAt":"2017-09-23T09:45:10Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This allows us to get rid of two write-only variables, one of them\nbeing a SHA1 buffer.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n refs/files-backend.c | 3 +--\n worktree.c           | 3 +--\n 2 files changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 32663a999e..a3134d23ab 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1676,13 +1676,12 @@ static int commit_ref_update(struct files_ref_store *refs,\n \t\t * check with HEAD only which should cover 99% of all usage\n \t\t * scenarios (even 100% of the default ones).\n \t\t */\n-\t\tstruct object_id head_oid;\n \t\tint head_flag;\n \t\tconst char *head_ref;\n \n \t\thead_ref = refs_resolve_ref_unsafe(&refs->base, \"HEAD\",\n \t\t\t\t\t\t   RESOLVE_REF_READING,\n-\t\t\t\t\t\t   head_oid.hash, &head_flag);\n+\t\t\t\t\t\t   NULL, &head_flag);\n \t\tif (head_ref && (head_flag & REF_ISSYMREF) &&\n \t\t    !strcmp(head_ref, lock->ref_name)) {\n \t\t\tstruct strbuf log_err = STRBUF_INIT;\ndiff --git a/worktree.c b/worktree.c\nindex 8aaeea0377..70015629dc 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -307,7 +307,6 @@ const struct worktree *find_shared_symref(const char *symref,\n \tfor (i = 0; worktrees[i]; i++) {\n \t\tstruct worktree *wt = worktrees[i];\n \t\tconst char *symref_target;\n-\t\tunsigned char sha1[20];\n \t\tstruct ref_store *refs;\n \t\tint flags;\n \n@@ -327,7 +326,7 @@ const struct worktree *find_shared_symref(const char *symref,\n \n \t\trefs = get_worktree_ref_store(wt);\n \t\tsymref_target = refs_resolve_ref_unsafe(refs, symref, 0,\n-\t\t\t\t\t\t\tsha1, &flags);\n+\t\t\t\t\t\t\tNULL, &flags);\n \t\tif ((flags & REF_ISSYMREF) && !strcmp(symref_target, target)) {\n \t\t\texisting = wt;\n \t\t\tbreak;\n-- \n2.14.1\n"},{"id":"328725","messageId":"01b1b3bd-c948-d849-9a03-7a8327fe5003@web.de","threadId":"46816","inReplyTo":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","subject":"[PATCH 3/3] refs: pass NULL to resolve_ref_unsafe() if hash is not needed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-23T09:45:04Z","receivedAt":"2017-09-23T09:45:17Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This allows us to get rid of some write-only variables, among them seven\nSHA1 buffers.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n branch.c                    |  3 +--\n builtin/commit.c            |  3 +--\n builtin/log.c               |  3 +--\n builtin/receive-pack.c      |  3 +--\n builtin/remote.c            |  3 +--\n builtin/submodule--helper.c |  6 ++----\n builtin/symbolic-ref.c      |  3 +--\n http-backend.c              |  3 +--\n log-tree.c                  |  3 +--\n refs.c                      |  6 ++----\n remote.c                    | 12 ++++--------\n revision.c                  |  3 +--\n transport.c                 |  3 +--\n upload-pack.c               |  3 +--\n 14 files changed, 19 insertions(+), 38 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 703ded69ca..4377ce2fb1 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -191,9 +191,8 @@ int validate_new_branchname(const char *name, struct strbuf *ref,\n \n \tif (!attr_only) {\n \t\tconst char *head;\n-\t\tstruct object_id oid;\n \n-\t\thead = resolve_ref_unsafe(\"HEAD\", 0, oid.hash, NULL);\n+\t\thead = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n \t\tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n \t\t\tdie(_(\"Cannot force update the current branch.\"));\n \t}\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 58f9747c2f..39d5b7f6c7 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1431,7 +1431,6 @@ static void print_summary(const char *prefix, const struct object_id *oid,\n \tstruct rev_info rev;\n \tstruct commit *commit;\n \tstruct strbuf format = STRBUF_INIT;\n-\tstruct object_id junk_oid;\n \tconst char *head;\n \tstruct pretty_print_context pctx = {0};\n \tstruct strbuf author_ident = STRBUF_INIT;\n@@ -1484,7 +1483,7 @@ static void print_summary(const char *prefix, const struct object_id *oid,\n \trev.diffopt.break_opt = 0;\n \tdiff_setup_done(&rev.diffopt);\n \n-\thead = resolve_ref_unsafe(\"HEAD\", 0, junk_oid.hash, NULL);\n+\thead = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n \tif (!strcmp(head, \"HEAD\"))\n \t\thead = _(\"detached HEAD\");\n \telse\ndiff --git a/builtin/log.c b/builtin/log.c\nindex f8cccbc964..d81a09051e 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1660,10 +1660,9 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)\n \t\t\tcheck_head = 1;\n \n \t\tif (check_head) {\n-\t\t\tstruct object_id oid;\n \t\t\tconst char *ref, *v;\n \t\t\tref = resolve_ref_unsafe(\"HEAD\", RESOLVE_REF_READING,\n-\t\t\t\t\t\t oid.hash, NULL);\n+\t\t\t\t\t\t NULL, NULL);\n \t\t\tif (ref && skip_prefix(ref, \"refs/heads/\", &v))\n \t\t\t\tbranch_name = xstrdup(v);\n \t\t\telse\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 52c63ebfdc..65d89078ab 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1207,11 +1207,10 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \tconst char *dst_name;\n \tstruct string_list_item *item;\n \tstruct command *dst_cmd;\n-\tunsigned char sha1[GIT_MAX_RAWSZ];\n \tint flag;\n \n \tstrbuf_addf(&buf, \"%s%s\", get_git_namespace(), cmd->ref_name);\n-\tdst_name = resolve_ref_unsafe(buf.buf, 0, sha1, &flag);\n+\tdst_name = resolve_ref_unsafe(buf.buf, 0, NULL, &flag);\n \tstrbuf_release(&buf);\n \n \tif (!(flag & REF_ISSYMREF))\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 33ba739332..4f5cac96b0 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -558,14 +558,13 @@ static int read_remote_branches(const char *refname,\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list_item *item;\n \tint flag;\n-\tstruct object_id orig_oid;\n \tconst char *symref;\n \n \tstrbuf_addf(&buf, \"refs/remotes/%s/\", rename->old);\n \tif (starts_with(refname, buf.buf)) {\n \t\titem = string_list_append(rename->remote_branches, xstrdup(refname));\n \t\tsymref = resolve_ref_unsafe(refname, RESOLVE_REF_READING,\n-\t\t\t\t\t    orig_oid.hash, &flag);\n+\t\t\t\t\t    NULL, &flag);\n \t\tif (flag & REF_ISSYMREF)\n \t\t\titem->util = xstrdup(symref);\n \t\telse\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 818fe74f0a..ea49710893 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -17,9 +17,8 @@\n static char *get_default_remote(void)\n {\n \tchar *dest = NULL, *ret;\n-\tunsigned char sha1[20];\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, NULL);\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n \n \tif (!refname)\n \t\tdie(_(\"No such ref: %s\"), \"HEAD\");\n@@ -1089,8 +1088,7 @@ static const char *remote_submodule_branch(const char *path)\n \t\treturn \"master\";\n \n \tif (!strcmp(branch, \".\")) {\n-\t\tunsigned char sha1[20];\n-\t\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, NULL);\n+\t\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n \n \t\tif (!refname)\n \t\t\tdie(_(\"No such ref: %s\"), \"HEAD\");\ndiff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c\nindex df75cb9d4a..17aabaa679 100644\n--- a/builtin/symbolic-ref.c\n+++ b/builtin/symbolic-ref.c\n@@ -12,9 +12,8 @@ static const char * const git_symbolic_ref_usage[] = {\n \n static int check_symref(const char *HEAD, int quiet, int shorten, int print)\n {\n-\tunsigned char sha1[20];\n \tint flag;\n-\tconst char *refname = resolve_ref_unsafe(HEAD, 0, sha1, &flag);\n+\tconst char *refname = resolve_ref_unsafe(HEAD, 0, NULL, &flag);\n \n \tif (!refname)\n \t\tdie(\"No such ref: %s\", HEAD);\ndiff --git a/http-backend.c b/http-backend.c\nindex 8076b1d5e5..b8ce960a35 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -486,10 +486,9 @@ static int show_head_ref(const char *refname, const struct object_id *oid,\n \tstruct strbuf *buf = cb_data;\n \n \tif (flag & REF_ISSYMREF) {\n-\t\tstruct object_id unused;\n \t\tconst char *target = resolve_ref_unsafe(refname,\n \t\t\t\t\t\t\tRESOLVE_REF_READING,\n-\t\t\t\t\t\t\tunused.hash, NULL);\n+\t\t\t\t\t\t\tNULL, NULL);\n \n \t\tif (target)\n \t\t\tstrbuf_addf(buf, \"ref: %s\\n\", strip_namespace(target));\ndiff --git a/log-tree.c b/log-tree.c\nindex 410ab4f02d..cea056234d 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -185,7 +185,6 @@ static const struct name_decoration *current_pointed_by_HEAD(const struct name_d\n {\n \tconst struct name_decoration *list, *head = NULL;\n \tconst char *branch_name = NULL;\n-\tstruct object_id unused;\n \tint rru_flags;\n \n \t/* First find HEAD */\n@@ -198,7 +197,7 @@ static const struct name_decoration *current_pointed_by_HEAD(const struct name_d\n \t\treturn NULL;\n \n \t/* Now resolve and find the matching current branch */\n-\tbranch_name = resolve_ref_unsafe(\"HEAD\", 0, unused.hash, &rru_flags);\n+\tbranch_name = resolve_ref_unsafe(\"HEAD\", 0, NULL, &rru_flags);\n \tif (!(rru_flags & REF_ISSYMREF))\n \t\treturn NULL;\n \ndiff --git a/refs.c b/refs.c\nindex 36439fdb54..97e33fdc7b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -239,8 +239,7 @@ int read_ref(const char *refname, unsigned char *sha1)\n \n int ref_exists(const char *refname)\n {\n-\tunsigned char sha1[20];\n-\treturn !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);\n+\treturn !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, NULL, NULL);\n }\n \n static int filter_refs(const char *refname, const struct object_id *oid,\n@@ -286,12 +285,11 @@ static int warn_if_dangling_symref(const char *refname, const struct object_id *\n {\n \tstruct warn_if_dangling_data *d = cb_data;\n \tconst char *resolves_to;\n-\tstruct object_id junk;\n \n \tif (!(flags & REF_ISSYMREF))\n \t\treturn 0;\n \n-\tresolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);\n+\tresolves_to = resolve_ref_unsafe(refname, 0, NULL, NULL);\n \tif (!resolves_to\n \t    || (d->refname\n \t\t? strcmp(resolves_to, d->refname)\ndiff --git a/remote.c b/remote.c\nindex 4113090069..b220f0dfc6 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -466,7 +466,6 @@ static void alias_all_urls(void)\n static void read_config(void)\n {\n \tstatic int loaded;\n-\tstruct object_id oid;\n \tint flag;\n \n \tif (loaded)\n@@ -475,7 +474,7 @@ static void read_config(void)\n \n \tcurrent_branch = NULL;\n \tif (startup_info->have_repository) {\n-\t\tconst char *head_ref = resolve_ref_unsafe(\"HEAD\", 0, oid.hash, &flag);\n+\t\tconst char *head_ref = resolve_ref_unsafe(\"HEAD\", 0, NULL, &flag);\n \t\tif (head_ref && (flag & REF_ISSYMREF) &&\n \t\t    skip_prefix(head_ref, \"refs/heads/\", &head_ref)) {\n \t\t\tcurrent_branch = make_branch(head_ref, 0);\n@@ -1105,10 +1104,9 @@ static struct ref *make_linked_ref(const char *name, struct ref ***tail)\n static char *guess_ref(const char *name, struct ref *peer)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tstruct object_id oid;\n \n \tconst char *r = resolve_ref_unsafe(peer->name, RESOLVE_REF_READING,\n-\t\t\t\t\t   oid.hash, NULL);\n+\t\t\t\t\t   NULL, NULL);\n \tif (!r)\n \t\treturn NULL;\n \n@@ -1166,12 +1164,11 @@ static int match_explicit(struct ref *src, struct ref *dst,\n \t\treturn -1;\n \n \tif (!dst_value) {\n-\t\tstruct object_id oid;\n \t\tint flag;\n \n \t\tdst_value = resolve_ref_unsafe(matched_src->name,\n \t\t\t\t\t       RESOLVE_REF_READING,\n-\t\t\t\t\t       oid.hash, &flag);\n+\t\t\t\t\t       NULL, &flag);\n \t\tif (!dst_value ||\n \t\t    ((flag & REF_ISSYMREF) &&\n \t\t     !starts_with(dst_value, \"refs/heads/\")))\n@@ -1792,10 +1789,9 @@ const char *branch_get_push(struct branch *branch, struct strbuf *err)\n \n static int ignore_symref_update(const char *refname)\n {\n-\tstruct object_id oid;\n \tint flag;\n \n-\tif (!resolve_ref_unsafe(refname, 0, oid.hash, &flag))\n+\tif (!resolve_ref_unsafe(refname, 0, NULL, &flag))\n \t\treturn 0; /* non-existing refs are OK */\n \treturn (flag & REF_ISSYMREF);\n }\ndiff --git a/revision.c b/revision.c\nindex f9a90d71d2..9582a727ca 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2263,11 +2263,10 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \n static void NORETURN diagnose_missing_default(const char *def)\n {\n-\tunsigned char sha1[20];\n \tint flags;\n \tconst char *refname;\n \n-\trefname = resolve_ref_unsafe(def, 0, sha1, &flags);\n+\trefname = resolve_ref_unsafe(def, 0, NULL, &flags);\n \tif (!refname || !(flags & REF_ISSYMREF) || (flags & REF_ISBROKEN))\n \t\tdie(_(\"your current branch appears to be broken\"));\n \ndiff --git a/transport.c b/transport.c\nindex d75ff0514d..fb8c01e57a 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -26,7 +26,6 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \t\tconst char *localname;\n \t\tconst char *tmp;\n \t\tconst char *remotename;\n-\t\tunsigned char sha[20];\n \t\tint flag = 0;\n \t\t/*\n \t\t * Check suitability for tracking. Must be successful /\n@@ -44,7 +43,7 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \t\tlocalname = ref->peer_ref->name;\n \t\tremotename = ref->name;\n \t\ttmp = resolve_ref_unsafe(localname, RESOLVE_REF_READING,\n-\t\t\t\t\t sha, &flag);\n+\t\t\t\t\t NULL, &flag);\n \t\tif (tmp && flag & REF_ISSYMREF &&\n \t\t\tstarts_with(tmp, \"refs/heads/\"))\n \t\t\tlocalname = tmp;\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 7efff2fbfd..06d822aad2 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -965,11 +965,10 @@ static int find_symref(const char *refname, const struct object_id *oid,\n {\n \tconst char *symref_target;\n \tstruct string_list_item *item;\n-\tstruct object_id unused;\n \n \tif ((flag & REF_ISSYMREF) == 0)\n \t\treturn 0;\n-\tsymref_target = resolve_ref_unsafe(refname, 0, unused.hash, &flag);\n+\tsymref_target = resolve_ref_unsafe(refname, 0, NULL, &flag);\n \tif (!symref_target || (flag & REF_ISSYMREF) == 0)\n \t\tdie(\"'%s' is a symref but it is not?\", refname);\n \titem = string_list_append(cb_data, refname);\n-- \n2.14.1\n"},{"id":"328757","messageId":"xmqq60c8j38t.fsf@gitster.mtv.corp.google.com","threadId":"46816","inReplyTo":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","subject":"Re: [PATCH 1/3] refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-09-24T01:26:42Z","receivedAt":"2017-09-24T01:26:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Allow callers of refs_resolve_ref_unsafe() to pass NULL if they don't\n> need the resolved hash value.  We already allow the same for the flags\n> parameter.  This new leniency is inherited by the various wrappers like\n> resolve_ref_unsafe().\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n\nThe patches all make sense, and I'd expect them to be something a\ncompetent contributor would write after coming to this realization:\nit seems that many many callers, when they have a ref that could be\na symbolic ref, want to learn (1) if it is a symref and (2) where it\npoints at, and nothing else, most notably they do not care what the\nobject pointed by the (symbolic) ref is.\n\nI wonder if we want a dedicated helper for them (perhaps about a\ndozen callers we have that fall into this pattern?) to call for the\nexact purpose, implemented as a thin-wrapper around the\nresolve_ref_unsafe() function, though.\n\nThanks.\n"},{"id":"328771","messageId":"047033a6-1e3d-103e-a406-cc89447f4327@web.de","threadId":"46816","inReplyTo":"xmqq60c8j38t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] refs: make sha1 output parameter of refs_resolve_ref_unsafe() optional","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-09-24T12:39:40Z","receivedAt":"2017-09-24T12:39:59Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.09.2017 um 03:26 schrieb Junio C Hamano:\n> I wonder if we want a dedicated helper for them (perhaps about a\n> dozen callers we have that fall into this pattern?) to call for the\n> exact purpose, implemented as a thin-wrapper around the\n> resolve_ref_unsafe() function, though.\n\nrefs_resolve_ref_unsafe() effectively has seven wrappers already:\n\n\tresolve_ref_unsafe()\n\trefs_resolve_refdup()\n\tresolve_refdup()\n\trefs_read_ref_full()\n\tread_ref_full()\n\tread_ref()\n\n\tref_exists()\n\nThe last one already supplies a dummy SHA1 buffer; for a handful of\nthe others we could add and use variants that do the same.  I feel\nthat this API would become a bit crowded, though.\n\nRené\n"},{"id":"329272","messageId":"366c778c-b587-de62-1d19-145d46fb1989@web.de","threadId":"46816","inReplyTo":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","subject":"[PATCH 4/3] refs: pass NULL to refs_resolve_refdup() if hash is not needed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-10-01T07:28:50Z","receivedAt":"2017-10-01T07:29:05Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This gets us rid of a write-only variable.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n refs/files-backend.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex fec77744b4..ed00ddab1a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2499,7 +2499,6 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \tstruct string_list affected_refnames = STRING_LIST_INIT_NODUP;\n \tchar *head_ref = NULL;\n \tint head_type;\n-\tstruct object_id head_oid;\n \tstruct files_transaction_backend_data *backend_data;\n \tstruct ref_transaction *packed_transaction = NULL;\n \n@@ -2556,7 +2555,7 @@ static int files_transaction_prepare(struct ref_store *ref_store,\n \t */\n \thead_ref = refs_resolve_refdup(ref_store, \"HEAD\",\n \t\t\t\t       RESOLVE_REF_NO_RECURSE,\n-\t\t\t\t       head_oid.hash, &head_type);\n+\t\t\t\t       NULL, &head_type);\n \n \tif (head_ref && !(head_type & REF_ISSYMREF)) {\n \t\tFREE_AND_NULL(head_ref);\n-- \n2.14.2\n\n"},{"id":"329273","messageId":"c18156ec-4c59-eb96-3177-0b1085b0a5c1@web.de","threadId":"46816","inReplyTo":"b89d36b5-0996-829b-a267-7ee4da9673dc@web.de","subject":"[PATCH 5/3] refs: pass NULL to resolve_refdup() if hash is not needed","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-10-01T07:29:03Z","receivedAt":"2017-10-01T07:29:18Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"This allows us to get rid of several write-only variables.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n builtin/checkout.c     | 3 +--\n builtin/receive-pack.c | 3 +--\n ref-filter.c           | 7 ++-----\n reflog-walk.c          | 6 ++----\n transport.c            | 3 +--\n wt-status.c            | 4 +---\n 6 files changed, 8 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 3345a0d16f..10751585ea 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1124,9 +1124,8 @@ static int checkout_branch(struct checkout_opts *opts,\n \n \tif (new->path && !opts->force_detach && !opts->new_branch &&\n \t    !opts->ignore_other_worktrees) {\n-\t\tstruct object_id oid;\n \t\tint flag;\n-\t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, oid.hash, &flag);\n+\t\tchar *head_ref = resolve_refdup(\"HEAD\", 0, NULL, &flag);\n \t\tif (head_ref &&\n \t\t    (!(flag & REF_ISSYMREF) || strcmp(head_ref, new->path)))\n \t\t\tdie_if_checked_out(new->path, 1);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 29a0f3b75f..cc48767405 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1458,7 +1458,6 @@ static void execute_commands(struct command *commands,\n {\n \tstruct check_connected_options opt = CHECK_CONNECTED_INIT;\n \tstruct command *cmd;\n-\tstruct object_id oid;\n \tstruct iterate_data data;\n \tstruct async muxer;\n \tint err_fd = 0;\n@@ -1515,7 +1514,7 @@ static void execute_commands(struct command *commands,\n \tcheck_aliased_updates(commands);\n \n \tfree(head_name_to_free);\n-\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", 0, oid.hash, NULL);\n+\thead_name = head_name_to_free = resolve_refdup(\"HEAD\", 0, NULL, NULL);\n \n \tif (use_atomic)\n \t\texecute_commands_atomic(commands, si);\ndiff --git a/ref-filter.c b/ref-filter.c\nindex bc591f4f3d..55323620ab 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -295,9 +295,7 @@ static void if_atom_parser(const struct ref_format *format, struct used_atom *at\n \n static void head_atom_parser(const struct ref_format *format, struct used_atom *atom, const char *arg)\n {\n-\tstruct object_id unused;\n-\n-\tatom->u.head = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, unused.hash, NULL);\n+\tatom->u.head = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, NULL, NULL);\n }\n \n static struct {\n@@ -1317,9 +1315,8 @@ static void populate_value(struct ref_array_item *ref)\n \tref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));\n \n \tif (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {\n-\t\tstruct object_id unused1;\n \t\tref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,\n-\t\t\t\t\t     unused1.hash, NULL);\n+\t\t\t\t\t     NULL, NULL);\n \t\tif (!ref->symref)\n \t\t\tref->symref = \"\";\n \t}\ndiff --git a/reflog-walk.c b/reflog-walk.c\nindex 74ebe5148f..842b2f77dc 100644\n--- a/reflog-walk.c\n+++ b/reflog-walk.c\n@@ -61,11 +61,10 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)\n \treflogs->ref = xstrdup(ref);\n \tfor_each_reflog_ent(ref, read_one_reflog, reflogs);\n \tif (reflogs->nr == 0) {\n-\t\tstruct object_id oid;\n \t\tconst char *name;\n \t\tvoid *name_to_free;\n \t\tname = name_to_free = resolve_refdup(ref, RESOLVE_REF_READING,\n-\t\t\t\t\t\t     oid.hash, NULL);\n+\t\t\t\t\t\t     NULL, NULL);\n \t\tif (name) {\n \t\t\tfor_each_reflog_ent(name, read_one_reflog, reflogs);\n \t\t\tfree(name_to_free);\n@@ -151,9 +150,8 @@ int add_reflog_for_walk(struct reflog_walk_info *info,\n \t\treflogs = item->util;\n \telse {\n \t\tif (*branch == '\\0') {\n-\t\t\tstruct object_id oid;\n \t\t\tfree(branch);\n-\t\t\tbranch = resolve_refdup(\"HEAD\", 0, oid.hash, NULL);\n+\t\t\tbranch = resolve_refdup(\"HEAD\", 0, NULL, NULL);\n \t\t\tif (!branch)\n \t\t\t\tdie (\"No current branch\");\n \ndiff --git a/transport.c b/transport.c\nindex fb8c01e57a..f1e2f61991 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -471,11 +471,10 @@ void transport_print_push_status(const char *dest, struct ref *refs,\n {\n \tstruct ref *ref;\n \tint n = 0;\n-\tstruct object_id head_oid;\n \tchar *head;\n \tint summary_width = transport_summary_width(refs);\n \n-\thead = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, head_oid.hash, NULL);\n+\thead = resolve_refdup(\"HEAD\", RESOLVE_REF_READING, NULL, NULL);\n \n \tif (verbose) {\n \t\tfor (ref = refs; ref; ref = ref->next)\ndiff --git a/wt-status.c b/wt-status.c\nindex 6f730ee8f2..29bc64cc02 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -121,15 +121,13 @@ static void status_printf_more(struct wt_status *s, const char *color,\n \n void wt_status_prepare(struct wt_status *s)\n {\n-\tstruct object_id oid;\n-\n \tmemset(s, 0, sizeof(*s));\n \tmemcpy(s->color_palette, default_wt_status_colors,\n \t       sizeof(default_wt_status_colors));\n \ts->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;\n \ts->use_color = -1;\n \ts->relative_paths = 1;\n-\ts->branch = resolve_refdup(\"HEAD\", 0, oid.hash, NULL);\n+\ts->branch = resolve_refdup(\"HEAD\", 0, NULL, NULL);\n \ts->reference = \"HEAD\";\n \ts->fp = stdout;\n \ts->index_file = get_index_file();\n-- \n2.14.2\n"},{"id":"329275","messageId":"xmqqfub3uv8p.fsf@gitster.mtv.corp.google.com","threadId":"46816","inReplyTo":"c18156ec-4c59-eb96-3177-0b1085b0a5c1@web.de","subject":"Re: [PATCH 5/3] refs: pass NULL to resolve_refdup() if hash is not needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-10-01T08:29:10Z","receivedAt":"2017-10-01T08:29:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> This allows us to get rid of several write-only variables.\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  builtin/checkout.c     | 3 +--\n>  builtin/receive-pack.c | 3 +--\n>  ref-filter.c           | 7 ++-----\n>  reflog-walk.c          | 6 ++----\n>  transport.c            | 3 +--\n>  wt-status.c            | 4 +---\n>  6 files changed, 8 insertions(+), 18 deletions(-)\n\nBoth patches looked good to me.\n\nIt's a bit surprising that we care so much about where a symbolic\nref points at, but it is understandable from the list of files\naffected.\n\n"}]}