{"thread":{"id":"41957","subject":"[PATCH 01/24] refs: move head_ref{,_submodule} to the common code","startedAt":"2016-04-07T19:02:47Z","lastAt":"2016-04-10T02:10:06Z","messageCount":32,"participants":["David Turner","Junio C Hamano","Michael Haggerty"],"isPatch":true,"patchVersion":1,"patchTotal":24},"messages":[{"id":"282876","messageId":"1460055791-23313-1-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":null,"subject":"[PATCH 00/24] Yet another pre-refs-backend series","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:47Z","receivedAt":"2016-04-07T19:02:47Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"We now have quite a large number of patches before we even get into\nthe meat of the pluggable refs backend series. So it's worth breaking\nthose out and getting them in before we get into the main series\n(which Michael Haggerty swants to redesign a bit anyway).\n\nThis set of patches should be applied on top of\njk/check-repository-format.\n\nMichael Haggerty has reviewed those of my patches which are in here\nexcept maybe:\n refs: on symref reflog expire, lock symref not referrent\nThis was the one from later in the series that was straightforward to\nmove to before the vtable; the other two were going to be harder to\nmove and can wait until after the vtable.\n\nI have reviewed all of Michael's patches.\n\n\nDavid Turner (5):\n refs: move head_ref{,_submodule} to the common code\n refs: move for_each_*ref* functions into common code\n files-backend: break out ref reading\n refs: move resolve_ref_unsafe into common code\n refs: on symref reflog expire, lock symref not referrent\n\nMichael Haggerty (19):\n t1430: test the output and error of some commands more carefully\n t1430: clean up broken refs/tags/shadow\n t1430: don't rely on symbolic-ref for creating broken symrefs\n t1430: test for-each-ref in the presence of badly-named refs\n t1430: improve test coverage of deletion of badly-named refs\n resolve_missing_loose_ref(): simplify semantics\n resolve_ref_unsafe(): use for loop to count up to MAXDEPTH\n resolve_ref_unsafe(): ensure flags is always set\n resolve_ref_1(): eliminate local variable\n resolve_ref_1(): reorder code\n resolve_ref_1(): eliminate local variable \"bad_name\"\n read_raw_ref(): manage own scratch space\n Inline resolve_ref_1() into resolve_ref_unsafe()\n read_raw_ref(): change flags parameter to unsigned int\n fsck_head_link(): remove unneeded flag variable\n cmd_merge(): remove unneeded flag variable\n checkout_paths(): remove unneeded flag variable\n check_aliased_update(): check that dst_name is non-NULL\n show_head_ref(): check the result of resolve_ref_namespace()\n\n builtin/checkout.c | 3 +-\n builtin/fsck.c | 3 +-\n builtin/merge.c | 4 +-\n builtin/receive-pack.c | 2 +-\n http-backend.c | 4 +-\n refs.c | 149 ++++++++++++++++++\n refs/files-backend.c | 406 ++++++++++++++++--------------------------------\n refs/refs-internal.h | 15 ++\n t/t1410-reflog.sh | 10 ++\n t/t1430-bad-ref-name.sh | 132 ++++++++++++++--\n 10 files changed, 440 insertions(+), 288 deletions(-)\n\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282875","messageId":"1460055791-23313-2-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 01/24] refs: move head_ref{,_submodule} to the common code","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:48Z","receivedAt":"2016-04-07T19:02:48Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"These don't use any backend-specific functions. These were previously\ndefined in terms of the do_head_ref helper function, but since they\nare otherwise identical, we don't need that function.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \n---\n refs.c | 23 +++++++++++++++++++++++\n refs/files-backend.c | 28 ----------------------------\n 2 files changed, 23 insertions(+), 28 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex b0e6ece..6b8c16c 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1080,3 +1080,26 @@ int rename_ref_available(const char *oldname, const char *newname)\n \tstrbuf_release(&err);\n \treturn ret;\n }\n+\n+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n+{\n+\tstruct object_id oid;\n+\tint flag;\n+\n+\tif (submodule) {\n+\t\tif (resolve_gitlink_ref(submodule, \"HEAD\", oid.hash) == 0)\n+\t\t\treturn fn(\"HEAD\", &oid, 0, cb_data);\n+\n+\t\treturn 0;\n+\t}\n+\n+\tif (!read_ref_full(\"HEAD\", RESOLVE_REF_READING, oid.hash, &flag))\n+\t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n+\n+\treturn 0;\n+}\n+\n+int head_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn head_ref_submodule(NULL, fn, cb_data);\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 81f68f8..c07dc41 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1745,34 +1745,6 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,\n \treturn do_for_each_entry(refs, base, do_one_ref, &data);\n }\n \n-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\tstruct object_id oid;\n-\tint flag;\n-\n-\tif (submodule) {\n-\t\tif (resolve_gitlink_ref(submodule, \"HEAD\", oid.hash) == 0)\n-\t\t\treturn fn(\"HEAD\", &oid, 0, cb_data);\n-\n-\t\treturn 0;\n-\t}\n-\n-\tif (!read_ref_full(\"HEAD\", RESOLVE_REF_READING, oid.hash, &flag))\n-\t\treturn fn(\"HEAD\", &oid, flag, cb_data);\n-\n-\treturn 0;\n-}\n-\n-int head_ref(each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_head_ref(NULL, fn, cb_data);\n-}\n-\n-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_head_ref(submodule, fn, cb_data);\n-}\n-\n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n \treturn do_for_each_ref(&ref_cache, \"\", fn, 0, 0, cb_data);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282878","messageId":"1460055791-23313-3-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 02/24] refs: move for_each_*ref* functions into common code","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:49Z","receivedAt":"2016-04-07T19:02:49Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Make do_for_each_ref take a submodule as an argument instead of a\nref_cache. Since all for_each_*ref* functions are defined in terms of\ndo_for_each_ref, we can then move them into the common code.\n\nLater, we can simply make do_for_each_ref into a backend function.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \n---\n refs.c | 52 +++++++++++++++++++++++++++++++++++++++++++\n refs/files-backend.c | 62 +++++-----------------------------------------------\n refs/refs-internal.h | 9 ++++++++\n 3 files changed, 66 insertions(+), 57 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 6b8c16c..f0feff7 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1103,3 +1103,55 @@ int head_ref(each_ref_fn fn, void *cb_data)\n {\n \treturn head_ref_submodule(NULL, fn, cb_data);\n }\n+\n+int for_each_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(NULL, \"\", fn, 0, 0, cb_data);\n+}\n+\n+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(submodule, \"\", fn, 0, 0, cb_data);\n+}\n+\n+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(NULL, prefix, fn, strlen(prefix), 0, cb_data);\n+}\n+\n+int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)\n+{\n+\tunsigned int flag = 0;\n+\n+\tif (broken)\n+\t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n+\treturn do_for_each_ref(NULL, prefix, fn, 0, flag, cb_data);\n+}\n+\n+int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n+\t\teach_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(submodule, prefix, fn, strlen(prefix), 0, cb_data);\n+}\n+\n+int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(NULL, git_replace_ref_base, fn,\n+\t\t\t strlen(git_replace_ref_base), 0, cb_data);\n+}\n+\n+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tint ret;\n+\tstrbuf_addf(&buf, \"%srefs/\", get_git_namespace());\n+\tret = do_for_each_ref(NULL, buf.buf, fn, 0, 0, cb_data);\n+\tstrbuf_release(&buf);\n+\treturn ret;\n+}\n+\n+int for_each_rawref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(NULL, \"\", fn, 0,\n+\t\t\t DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c07dc41..9676ec2 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -513,9 +513,6 @@ static void sort_ref_dir(struct ref_dir *dir)\n \tdir->sorted = dir->nr = i;\n }\n \n-/* Include broken references in a do_for_each_ref*() iteration: */\n-#define DO_FOR_EACH_INCLUDE_BROKEN 0x01\n-\n /*\n * Return true iff the reference described by entry can be resolved to\n * an object in the database. Emit a warning if the referred-to\n@@ -1727,10 +1724,13 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,\n * value, stop the iteration and return that value; otherwise, return\n * 0.\n */\n-static int do_for_each_ref(struct ref_cache *refs, const char *base,\n-\t\t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n+int do_for_each_ref(const char *submodule, const char *base,\n+\t\t each_ref_fn fn, int trim, int flags, void *cb_data)\n {\n \tstruct ref_entry_cb data;\n+\tstruct ref_cache *refs;\n+\n+\trefs = get_ref_cache(submodule);\n \tdata.base = base;\n \tdata.trim = trim;\n \tdata.flags = flags;\n@@ -1745,58 +1745,6 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,\n \treturn do_for_each_entry(refs, base, do_one_ref, &data);\n }\n \n-int for_each_ref(each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(&ref_cache, \"\", fn, 0, 0, cb_data);\n-}\n-\n-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(get_ref_cache(submodule), \"\", fn, 0, 0, cb_data);\n-}\n-\n-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);\n-}\n-\n-int for_each_fullref_in(const char *prefix, each_ref_fn fn, void *cb_data, unsigned int broken)\n-{\n-\tunsigned int flag = 0;\n-\n-\tif (broken)\n-\t\tflag = DO_FOR_EACH_INCLUDE_BROKEN;\n-\treturn do_for_each_ref(&ref_cache, prefix, fn, 0, flag, cb_data);\n-}\n-\n-int for_each_ref_in_submodule(const char *submodule, const char *prefix,\n-\t\teach_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);\n-}\n-\n-int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(&ref_cache, git_replace_ref_base, fn,\n-\t\t\t strlen(git_replace_ref_base), 0, cb_data);\n-}\n-\n-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)\n-{\n-\tstruct strbuf buf = STRBUF_INIT;\n-\tint ret;\n-\tstrbuf_addf(&buf, \"%srefs/\", get_git_namespace());\n-\tret = do_for_each_ref(&ref_cache, buf.buf, fn, 0, 0, cb_data);\n-\tstrbuf_release(&buf);\n-\treturn ret;\n-}\n-\n-int for_each_rawref(each_ref_fn fn, void *cb_data)\n-{\n-\treturn do_for_each_ref(&ref_cache, \"\", fn, 0,\n-\t\t\t DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n-}\n-\n static void unlock_ref(struct ref_lock *lock)\n {\n \t/* Do not free lock->lk -- atexit() still looks at them */\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex c7dded3..92aae80 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -197,4 +197,13 @@ const char *find_descendant_ref(const char *dirname,\n \n int rename_ref_available(const char *oldname, const char *newname);\n \n+\n+/* Include broken references in a do_for_each_ref*() iteration: */\n+#define DO_FOR_EACH_INCLUDE_BROKEN 0x01\n+\n+/*\n+ * The common backend for the for_each_*ref* functions\n+ */\n+int do_for_each_ref(const char *submodule, const char *base,\n+\t\t each_ref_fn fn, int trim, int flags, void *cb_data);\n #endif /* REFS_REFS_INTERNAL_H */\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282895","messageId":"1460055791-23313-4-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 03/24] t1430: test the output and error of some commands more carefully","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:50Z","receivedAt":"2016-04-07T19:02:50Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nSigned-off-by: Michael Haggerty \n---\n t/t1430-bad-ref-name.sh | 18 ++++++++++++------\n 1 file changed, 12 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex c465abe..005e2b1 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -42,7 +42,7 @@ test_expect_success 'git branch shows badly named ref as warning' '\n \tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n \ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n \tgit branch >output 2>error &&\n-\tgrep -e \"broken\\.\\.\\.ref\" error &&\n+\ttest_i18ngrep -e \"ignoring ref with broken name refs/heads/broken\\.\\.\\.ref\" error &&\n \t! grep -e \"broken\\.\\.\\.ref\" output\n '\n \n@@ -152,21 +152,25 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '\n \tgit rev-parse --verify one >expect &&\n \tgit rev-parse --verify shadow >actual 2>err &&\n \ttest_cmp expect actual &&\n-\ttest_i18ngrep \"ignoring.*refs/tags/shadow\" err\n+\ttest_i18ngrep \"ignoring dangling symref refs/tags/shadow\" err\n '\n \n test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n \tgit symbolic-ref refs/heads/badname refs/heads/broken...ref &&\n \ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n \ttest_path_is_file .git/refs/heads/badname &&\n-\tgit update-ref --no-deref -d refs/heads/badname &&\n-\ttest_path_is_missing .git/refs/heads/badname\n+\tgit update-ref --no-deref -d refs/heads/badname >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/badname &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error\n '\n \n test_expect_success 'update-ref -d can delete broken name' '\n \tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n \ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n-\tgit update-ref -d refs/heads/broken...ref &&\n+\tgit update-ref -d refs/heads/broken...ref >output 2>error &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error &&\n \tgit branch >output 2>error &&\n \t! grep -e \"broken\\.\\.\\.ref\" error &&\n \t! grep -e \"broken\\.\\.\\.ref\" output\n@@ -175,7 +179,9 @@ test_expect_success 'update-ref -d can delete broken name' '\n test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '\n \techo precious >.git/my-private-file &&\n \techo precious >expect &&\n-\ttest_must_fail git update-ref -d my-private-file &&\n+\ttest_must_fail git update-ref -d my-private-file >output 2>error &&\n+\ttest_must_be_empty output &&\n+\ttest_i18ngrep -e \"cannot lock .*: unable to resolve reference\" error &&\n \ttest_cmp expect .git/my-private-file\n '\n \n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282894","messageId":"1460055791-23313-5-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 04/24] t1430: clean up broken refs/tags/shadow","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:51Z","receivedAt":"2016-04-07T19:02:51Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nSigned-off-by: Michael Haggerty \n---\n t/t1430-bad-ref-name.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex 005e2b1..cb815ab 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -148,7 +148,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '\n \tgit branch shadow one &&\n \tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n \tgit symbolic-ref refs/tags/shadow refs/heads/broken...ref &&\n-\n+\ttest_when_finished \"rm -f .git/refs/tags/shadow\" &&\n \tgit rev-parse --verify one >expect &&\n \tgit rev-parse --verify shadow >actual 2>err &&\n \ttest_cmp expect actual &&\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282893","messageId":"1460055791-23313-6-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 05/24] t1430: don't rely on symbolic-ref for creating broken symrefs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:52Z","receivedAt":"2016-04-07T19:02:52Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIt's questionable whether it should even work.\n\nSigned-off-by: Michael Haggerty \n---\n t/t1430-bad-ref-name.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex cb815ab..a963951 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -147,7 +147,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '\n \ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n \tgit branch shadow one &&\n \tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n-\tgit symbolic-ref refs/tags/shadow refs/heads/broken...ref &&\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/tags/shadow &&\n \ttest_when_finished \"rm -f .git/refs/tags/shadow\" &&\n \tgit rev-parse --verify one >expect &&\n \tgit rev-parse --verify shadow >actual 2>err &&\n@@ -156,7 +156,7 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '\n '\n \n test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n-\tgit symbolic-ref refs/heads/badname refs/heads/broken...ref &&\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n \ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n \ttest_path_is_file .git/refs/heads/badname &&\n \tgit update-ref --no-deref -d refs/heads/badname >output 2>error &&\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282879","messageId":"1460055791-23313-7-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 06/24] t1430: test for-each-ref in the presence of badly-named refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:53Z","receivedAt":"2016-04-07T19:02:53Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nSigned-off-by: Michael Haggerty \n---\n t/t1430-bad-ref-name.sh | 16 ++++++++++++++++\n 1 file changed, 16 insertions(+)\n\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex a963951..612cc32 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -155,6 +155,22 @@ test_expect_success 'rev-parse skips symref pointing to broken name' '\n \ttest_i18ngrep \"ignoring dangling symref refs/tags/shadow\" err\n '\n \n+test_expect_success 'for-each-ref emits warnings for broken names' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\tprintf \"ref: refs/heads/master\\n\" >.git/refs/heads/broken...symref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...symref\" &&\n+\tgit for-each-ref >output 2>error &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output &&\n+\t! grep -e \"badname\" output &&\n+\t! grep -e \"broken\\.\\.\\.symref\" output &&\n+\ttest_i18ngrep \"ignoring ref with broken name refs/heads/broken\\.\\.\\.ref\" error &&\n+\ttest_i18ngrep \"ignoring broken ref refs/heads/badname\" error &&\n+\ttest_i18ngrep \"ignoring ref with broken name refs/heads/broken\\.\\.\\.symref\" error\n+'\n+\n test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n \tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n \ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282880","messageId":"1460055791-23313-8-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 07/24] t1430: improve test coverage of deletion of badly-named refs","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:54Z","receivedAt":"2016-04-07T19:02:54Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nCheck \"branch -d broken...ref\"\n\nCheck various combinations of\n\n* Deleting using \"update-ref -d\"\n* Deleting using \"update-ref --no-deref -d\"\n* Deleting using \"branch -d\"\n\nin the following combinations of symref -> ref:\n\n* badname -> broken...ref\n* badname -> broken...ref (dangling)\n* broken...symref -> master\n* broken...symref -> idonotexist (dangling)\n\nSigned-off-by: Michael Haggerty \n---\n t/t1430-bad-ref-name.sh | 104 ++++++++++++++++++++++++++++++++++++++++++++----\n 1 file changed, 96 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t1430-bad-ref-name.sh b/t/t1430-bad-ref-name.sh\nindex 612cc32..25ddab4 100755\n--- a/t/t1430-bad-ref-name.sh\n+++ b/t/t1430-bad-ref-name.sh\n@@ -171,25 +171,113 @@ test_expect_success 'for-each-ref emits warnings for broken names' '\n \ttest_i18ngrep \"ignoring ref with broken name refs/heads/broken\\.\\.\\.symref\" error\n '\n \n-test_expect_success 'update-ref --no-deref -d can delete reference to broken name' '\n+test_expect_success 'update-ref -d can delete broken name' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit update-ref -d refs/heads/broken...ref >output 2>error &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error &&\n+\tgit branch >output 2>error &&\n+\t! grep -e \"broken\\.\\.\\.ref\" error &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'branch -d can delete broken name' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tgit branch -d broken...ref >output 2>error &&\n+\ttest_i18ngrep \"Deleted branch broken...ref (was broken)\" output &&\n+\ttest_must_be_empty error &&\n+\tgit branch >output 2>error &&\n+\t! grep -e \"broken\\.\\.\\.ref\" error &&\n+\t! grep -e \"broken\\.\\.\\.ref\" output\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete symref to broken name' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n \tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n \ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n-\ttest_path_is_file .git/refs/heads/badname &&\n \tgit update-ref --no-deref -d refs/heads/badname >output 2>error &&\n \ttest_path_is_missing .git/refs/heads/badname &&\n \ttest_must_be_empty output &&\n \ttest_must_be_empty error\n '\n \n-test_expect_success 'update-ref -d can delete broken name' '\n+test_expect_success 'branch -d can delete symref to broken name' '\n \tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n \ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n-\tgit update-ref -d refs/heads/broken...ref >output 2>error &&\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\tgit branch -d badname >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/badname &&\n+\ttest_i18ngrep \"Deleted branch badname (was refs/heads/broken\\.\\.\\.ref)\" output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete dangling symref to broken name' '\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\tgit update-ref --no-deref -d refs/heads/badname >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/badname &&\n \ttest_must_be_empty output &&\n-\ttest_must_be_empty error &&\n-\tgit branch >output 2>error &&\n-\t! grep -e \"broken\\.\\.\\.ref\" error &&\n-\t! grep -e \"broken\\.\\.\\.ref\" output\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'branch -d can delete dangling symref to broken name' '\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\tgit branch -d badname >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/badname &&\n+\ttest_i18ngrep \"Deleted branch badname (was refs/heads/broken\\.\\.\\.ref)\" output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'update-ref -d can delete broken name through symref' '\n+\tcp .git/refs/heads/master .git/refs/heads/broken...ref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...ref\" &&\n+\tprintf \"ref: refs/heads/broken...ref\\n\" >.git/refs/heads/badname &&\n+\ttest_when_finished \"rm -f .git/refs/heads/badname\" &&\n+\tgit update-ref -d refs/heads/badname >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/broken...ref &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete symref with broken name' '\n+\tprintf \"ref: refs/heads/master\\n\" >.git/refs/heads/broken...symref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...symref\" &&\n+\tgit update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/broken...symref &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'branch -d can delete symref with broken name' '\n+\tprintf \"ref: refs/heads/master\\n\" >.git/refs/heads/broken...symref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...symref\" &&\n+\tgit branch -d broken...symref >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/broken...symref &&\n+\ttest_i18ngrep \"Deleted branch broken...symref (was refs/heads/master)\" output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'update-ref --no-deref -d can delete dangling symref with broken name' '\n+\tprintf \"ref: refs/heads/idonotexist\\n\" >.git/refs/heads/broken...symref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...symref\" &&\n+\tgit update-ref --no-deref -d refs/heads/broken...symref >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/broken...symref &&\n+\ttest_must_be_empty output &&\n+\ttest_must_be_empty error\n+'\n+\n+test_expect_success 'branch -d can delete dangling symref with broken name' '\n+\tprintf \"ref: refs/heads/idonotexist\\n\" >.git/refs/heads/broken...symref &&\n+\ttest_when_finished \"rm -f .git/refs/heads/broken...symref\" &&\n+\tgit branch -d broken...symref >output 2>error &&\n+\ttest_path_is_missing .git/refs/heads/broken...symref &&\n+\ttest_i18ngrep \"Deleted branch broken...symref (was refs/heads/idonotexist)\" output &&\n+\ttest_must_be_empty error\n '\n \n test_expect_success 'update-ref -d cannot delete non-ref in .git dir' '\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282877","messageId":"1460055791-23313-9-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 08/24] resolve_missing_loose_ref(): simplify semantics","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:55Z","receivedAt":"2016-04-07T19:02:55Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nMake resolve_missing_loose_ref() only responsible for looking up a\npacked reference, without worrying about whether we want to read or\nwrite the reference and without setting errno on failure. Move the other\nlogic to the caller.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 24 ++++++++++--------------\n 1 file changed, 10 insertions(+), 14 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 9676ec2..c0cf6fd 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1368,11 +1368,9 @@ static struct ref_entry *get_packed_ref(const char *refname)\n }\n \n /*\n- * A loose ref file doesn't exist; check for a packed ref. The\n- * options are forwarded from resolve_safe_unsafe().\n+ * A loose ref file doesn't exist; check for a packed ref.\n */\n static int resolve_missing_loose_ref(const char *refname,\n-\t\t\t\t int resolve_flags,\n \t\t\t\t unsigned char *sha1,\n \t\t\t\t int *flags)\n {\n@@ -1389,14 +1387,8 @@ static int resolve_missing_loose_ref(const char *refname,\n \t\t\t*flags |= REF_ISPACKED;\n \t\treturn 0;\n \t}\n-\t/* The reference is not a packed reference, either. */\n-\tif (resolve_flags & RESOLVE_REF_READING) {\n-\t\terrno = ENOENT;\n-\t\treturn -1;\n-\t} else {\n-\t\thashclr(sha1);\n-\t\treturn 0;\n-\t}\n+\t/* refname is not a packed reference. */\n+\treturn -1;\n }\n \n /* This function needs to return a meaningful errno on failure */\n@@ -1461,9 +1453,13 @@ static const char *resolve_ref_1(const char *refname,\n \t\tif (lstat(path, &st) < 0) {\n \t\t\tif (errno != ENOENT)\n \t\t\t\treturn NULL;\n-\t\t\tif (resolve_missing_loose_ref(refname, resolve_flags,\n-\t\t\t\t\t\t sha1, flags))\n-\t\t\t\treturn NULL;\n+\t\t\tif (resolve_missing_loose_ref(refname, sha1, flags)) {\n+\t\t\t\tif (resolve_flags & RESOLVE_REF_READING) {\n+\t\t\t\t\terrno = ENOENT;\n+\t\t\t\t\treturn NULL;\n+\t\t\t\t}\n+\t\t\t\thashclr(sha1);\n+\t\t\t}\n \t\t\tif (bad_name) {\n \t\t\t\thashclr(sha1);\n \t\t\t\tif (flags)\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282896","messageId":"1460055791-23313-10-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 09/24] resolve_ref_unsafe(): use for loop to count up to MAXDEPTH","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:56Z","receivedAt":"2016-04-07T19:02:56Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nThe loop's there anyway; we might as well use it.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c0cf6fd..101abba 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1400,8 +1400,8 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\t struct strbuf *sb_path,\n \t\t\t\t struct strbuf *sb_contents)\n {\n-\tint depth = MAXDEPTH;\n \tint bad_name = 0;\n+\tint symref_count;\n \n \tif (flags)\n \t\t*flags = 0;\n@@ -1425,17 +1425,13 @@ static const char *resolve_ref_1(const char *refname,\n \t\t */\n \t\tbad_name = 1;\n \t}\n-\tfor (;;) {\n+\n+\tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n \t\tconst char *path;\n \t\tstruct stat st;\n \t\tchar *buf;\n \t\tint fd;\n \n-\t\tif (--depth < 0) {\n-\t\t\terrno = ELOOP;\n-\t\t\treturn NULL;\n-\t\t}\n-\n \t\tstrbuf_reset(sb_path);\n \t\tstrbuf_git_path(sb_path, \"%s\", refname);\n \t\tpath = sb_path->buf;\n@@ -1566,6 +1562,9 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\tbad_name = 1;\n \t\t}\n \t}\n+\n+\terrno = ELOOP;\n+\treturn NULL;\n }\n \n const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282899","messageId":"1460055791-23313-11-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 10/24] resolve_ref_unsafe(): ensure flags is always set","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:57Z","receivedAt":"2016-04-07T19:02:57Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIf the caller passes flags==NULL, then set it to point at a local\nscratch variable. This removes the need for a lot of \"if (flags)\" guards\nin resolve_ref_1() and resolve_missing_loose_ref().\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 31 +++++++++++++------------------\n 1 file changed, 13 insertions(+), 18 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 101abba..067ce1c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1383,8 +1383,7 @@ static int resolve_missing_loose_ref(const char *refname,\n \tentry = get_packed_ref(refname);\n \tif (entry) {\n \t\thashcpy(sha1, entry->u.value.oid.hash);\n-\t\tif (flags)\n-\t\t\t*flags |= REF_ISPACKED;\n+\t\t*flags |= REF_ISPACKED;\n \t\treturn 0;\n \t}\n \t/* refname is not a packed reference. */\n@@ -1403,12 +1402,10 @@ static const char *resolve_ref_1(const char *refname,\n \tint bad_name = 0;\n \tint symref_count;\n \n-\tif (flags)\n-\t\t*flags = 0;\n+\t*flags = 0;\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\tif (flags)\n-\t\t\t*flags |= REF_BAD_NAME;\n+\t\t*flags |= REF_BAD_NAME;\n \n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n \t\t !refname_is_safe(refname)) {\n@@ -1458,8 +1455,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t}\n \t\t\tif (bad_name) {\n \t\t\t\thashclr(sha1);\n-\t\t\t\tif (flags)\n-\t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t}\n \t\t\treturn refname;\n \t\t}\n@@ -1478,8 +1474,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t !check_refname_format(sb_contents->buf, 0)) {\n \t\t\t\tstrbuf_swap(sb_refname, sb_contents);\n \t\t\t\trefname = sb_refname->buf;\n-\t\t\t\tif (flags)\n-\t\t\t\t\t*flags |= REF_ISSYMREF;\n+\t\t\t\t*flags |= REF_ISSYMREF;\n \t\t\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n \t\t\t\t\thashclr(sha1);\n \t\t\t\t\treturn refname;\n@@ -1526,20 +1521,17 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t */\n \t\t\tif (get_sha1_hex(sb_contents->buf, sha1) ||\n \t\t\t (sb_contents->buf[40] != '\\0' && !isspace(sb_contents->buf[40]))) {\n-\t\t\t\tif (flags)\n-\t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n \t\t\tif (bad_name) {\n \t\t\t\thashclr(sha1);\n-\t\t\t\tif (flags)\n-\t\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t}\n \t\t\treturn refname;\n \t\t}\n-\t\tif (flags)\n-\t\t\t*flags |= REF_ISSYMREF;\n+\t\t*flags |= REF_ISSYMREF;\n \t\tbuf = sb_contents->buf + 4;\n \t\twhile (isspace(*buf))\n \t\t\tbuf++;\n@@ -1551,8 +1543,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\treturn refname;\n \t\t}\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n-\t\t\tif (flags)\n-\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t*flags |= REF_ISBROKEN;\n \n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n \t\t\t !refname_is_safe(buf)) {\n@@ -1573,8 +1564,12 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n \tstruct strbuf sb_contents = STRBUF_INIT;\n \tstruct strbuf sb_path = STRBUF_INIT;\n+\tint unused_flags;\n \tconst char *ret;\n \n+\tif (!flags)\n+\t\tflags = &unused_flags;\n+\n \tret = resolve_ref_1(refname, resolve_flags, sha1, flags,\n \t\t\t &sb_refname, &sb_path, &sb_contents);\n \tstrbuf_release(&sb_path);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282885","messageId":"1460055791-23313-12-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 11/24] resolve_ref_1(): eliminate local variable","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:58Z","receivedAt":"2016-04-07T19:02:58Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIn place of `buf`, use `refname`, which is anyway a better description\nof what is being pointed at.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 13 ++++++-------\n 1 file changed, 6 insertions(+), 7 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 067ce1c..69ec903 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1426,7 +1426,6 @@ static const char *resolve_ref_1(const char *refname,\n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n \t\tconst char *path;\n \t\tstruct stat st;\n-\t\tchar *buf;\n \t\tint fd;\n \n \t\tstrbuf_reset(sb_path);\n@@ -1532,21 +1531,21 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\treturn refname;\n \t\t}\n \t\t*flags |= REF_ISSYMREF;\n-\t\tbuf = sb_contents->buf + 4;\n-\t\twhile (isspace(*buf))\n-\t\t\tbuf++;\n+\t\trefname = sb_contents->buf + 4;\n+\t\twhile (isspace(*refname))\n+\t\t\trefname++;\n \t\tstrbuf_reset(sb_refname);\n-\t\tstrbuf_addstr(sb_refname, buf);\n+\t\tstrbuf_addstr(sb_refname, refname);\n \t\trefname = sb_refname->buf;\n \t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n \t\t\thashclr(sha1);\n \t\t\treturn refname;\n \t\t}\n-\t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n+\t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n \t\t\t*flags |= REF_ISBROKEN;\n \n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(buf)) {\n+\t\t\t !refname_is_safe(refname)) {\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282882","messageId":"1460055791-23313-13-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 12/24] resolve_ref_1(): reorder code","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:02:59Z","receivedAt":"2016-04-07T19:02:59Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nThere is no need to adjust *flags if we're just about to fail.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 69ec903..60f1493 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1542,13 +1542,13 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\treturn refname;\n \t\t}\n \t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\t\t*flags |= REF_ISBROKEN;\n-\n \t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n \t\t\t !refname_is_safe(refname)) {\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n+\n+\t\t\t*flags |= REF_ISBROKEN;\n \t\t\tbad_name = 1;\n \t\t}\n \t}\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282884","messageId":"1460055791-23313-14-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 13/24] resolve_ref_1(): eliminate local variable \"bad_name\"","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:00Z","receivedAt":"2016-04-07T19:03:00Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nWe can use (*flags & REF_BAD_NAME) for that purpose.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 13 +++++--------\n 1 file changed, 5 insertions(+), 8 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 60f1493..b865ba5 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1399,19 +1399,17 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\t struct strbuf *sb_path,\n \t\t\t\t struct strbuf *sb_contents)\n {\n-\tint bad_name = 0;\n \tint symref_count;\n \n \t*flags = 0;\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\t*flags |= REF_BAD_NAME;\n-\n \t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n \t\t !refname_is_safe(refname)) {\n \t\t\terrno = EINVAL;\n \t\t\treturn NULL;\n \t\t}\n+\n \t\t/*\n \t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n \t\t * missing refs and refs that were present but invalid,\n@@ -1420,7 +1418,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t * We don't know whether the ref exists, so don't set\n \t\t * REF_ISBROKEN yet.\n \t\t */\n-\t\tbad_name = 1;\n+\t\t*flags |= REF_BAD_NAME;\n \t}\n \n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n@@ -1452,7 +1450,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\t}\n \t\t\t\thashclr(sha1);\n \t\t\t}\n-\t\t\tif (bad_name) {\n+\t\t\tif (*flags & REF_BAD_NAME) {\n \t\t\t\thashclr(sha1);\n \t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t}\n@@ -1524,7 +1522,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\terrno = EINVAL;\n \t\t\t\treturn NULL;\n \t\t\t}\n-\t\t\tif (bad_name) {\n+\t\t\tif (*flags & REF_BAD_NAME) {\n \t\t\t\thashclr(sha1);\n \t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t}\n@@ -1548,8 +1546,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\treturn NULL;\n \t\t\t}\n \n-\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\tbad_name = 1;\n+\t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n \t\t}\n \t}\n \n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282883","messageId":"1460055791-23313-15-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 14/24] files-backend: break out ref reading","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:01Z","receivedAt":"2016-04-07T19:03:01Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Refactor resolve_ref_1 in terms of a new function read_raw_ref, which\nis responsible for reading ref data from the ref storage.\n\nLater, we will make read_raw_ref a pluggable backend function, and make\nresolve_ref_unsafe common.\n\nSigned-off-by: David Turner \nHelped-by: Duy Nguyen \nSigned-off-by: Junio C Hamano \nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 244 ++++++++++++++++++++++++++++++---------------------\n 1 file changed, 145 insertions(+), 99 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex b865ba5..d51e778 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1390,6 +1390,141 @@ static int resolve_missing_loose_ref(const char *refname,\n \treturn -1;\n }\n \n+/*\n+ * Read a raw ref from the filesystem or packed refs file.\n+ *\n+ * If the ref is a sha1, fill in sha1 and return 0.\n+ *\n+ * If the ref is symbolic, fill in *symref with the referrent\n+ * (e.g. \"refs/heads/master\") and return 0. The caller is responsible\n+ * for validating the referrent. Set REF_ISSYMREF in flags.\n+ *\n+ * If the ref doesn't exist, set errno to ENOENT and return -1.\n+ *\n+ * If the ref exists but is neither a symbolic ref nor a sha1, it is\n+ * broken. Set REF_ISBROKEN in flags, set errno to EINVAL, and return\n+ * -1.\n+ *\n+ * If there is another error reading the ref, set errno appropriately and\n+ * return -1.\n+ *\n+ * Backend-specific flags might be set in flags as well, regardless of\n+ * outcome.\n+ *\n+ * sb_path is workspace: the caller should allocate and free it.\n+ *\n+ * It is OK for refname to point into symref. In this case:\n+ * - if the function succeeds with REF_ISSYMREF, symref will be\n+ * overwritten and the memory pointed to by refname might be changed\n+ * or even freed.\n+ * - in all other cases, symref will be untouched, and therefore\n+ * refname will still be valid and unchanged.\n+ */\n+static int read_raw_ref(const char *refname, unsigned char *sha1,\n+\t\t\tstruct strbuf *symref, struct strbuf *sb_path,\n+\t\t\tstruct strbuf *sb_contents, int *flags)\n+{\n+\tconst char *path;\n+\tconst char *buf;\n+\tstruct stat st;\n+\tint fd;\n+\n+\tstrbuf_reset(sb_path);\n+\tstrbuf_git_path(sb_path, \"%s\", refname);\n+\tpath = sb_path->buf;\n+\n+stat_ref:\n+\t/*\n+\t * We might have to loop back here to avoid a race\n+\t * condition: first we lstat() the file, then we try\n+\t * to read it as a link or as a file. But if somebody\n+\t * changes the type of the file (file <-> directory\n+\t * <-> symlink) between the lstat() and reading, then\n+\t * we don't want to report that as an error but rather\n+\t * try again starting with the lstat().\n+\t */\n+\n+\tif (lstat(path, &st) < 0) {\n+\t\tif (errno != ENOENT)\n+\t\t\treturn -1;\n+\t\tif (resolve_missing_loose_ref(refname, sha1, flags)) {\n+\t\t\terrno = ENOENT;\n+\t\t\treturn -1;\n+\t\t}\n+\t\treturn 0;\n+\t}\n+\n+\t/* Follow \"normalized\" - ie \"refs/..\" symlinks by hand */\n+\tif (S_ISLNK(st.st_mode)) {\n+\t\tstrbuf_reset(sb_contents);\n+\t\tif (strbuf_readlink(sb_contents, path, 0) < 0) {\n+\t\t\tif (errno == ENOENT || errno == EINVAL)\n+\t\t\t\t/* inconsistent with lstat; retry */\n+\t\t\t\tgoto stat_ref;\n+\t\t\telse\n+\t\t\t\treturn -1;\n+\t\t}\n+\t\tif (starts_with(sb_contents->buf, \"refs/\") &&\n+\t\t !check_refname_format(sb_contents->buf, 0)) {\n+\t\t\tstrbuf_swap(sb_contents, symref);\n+\t\t\t*flags |= REF_ISSYMREF;\n+\t\t\treturn 0;\n+\t\t}\n+\t}\n+\n+\t/* Is it a directory? */\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terrno = EISDIR;\n+\t\treturn -1;\n+\t}\n+\n+\t/*\n+\t * Anything else, just open it and try to use it as\n+\t * a ref\n+\t */\n+\tfd = open(path, O_RDONLY);\n+\tif (fd < 0) {\n+\t\tif (errno == ENOENT)\n+\t\t\t/* inconsistent with lstat; retry */\n+\t\t\tgoto stat_ref;\n+\t\telse\n+\t\t\treturn -1;\n+\t}\n+\tstrbuf_reset(sb_contents);\n+\tif (strbuf_read(sb_contents, fd, 256) < 0) {\n+\t\tint save_errno = errno;\n+\t\tclose(fd);\n+\t\terrno = save_errno;\n+\t\treturn -1;\n+\t}\n+\tclose(fd);\n+\tstrbuf_rtrim(sb_contents);\n+\tbuf = sb_contents->buf;\n+\tif (starts_with(buf, \"ref:\")) {\n+\t\tbuf += 4;\n+\t\twhile (isspace(*buf))\n+\t\t\tbuf++;\n+\n+\t\tstrbuf_reset(symref);\n+\t\tstrbuf_addstr(symref, buf);\n+\t\t*flags |= REF_ISSYMREF;\n+\t\treturn 0;\n+\t}\n+\n+\t/*\n+\t * Please note that FETCH_HEAD has additional\n+\t * data after the sha.\n+\t */\n+\tif (get_sha1_hex(buf, sha1) ||\n+\t (buf[40] != '\\0' && !isspace(buf[40]))) {\n+\t\t*flags |= REF_ISBROKEN;\n+\t\terrno = EINVAL;\n+\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n /* This function needs to return a meaningful errno on failure */\n static const char *resolve_ref_1(const char *refname,\n \t\t\t\t int resolve_flags,\n@@ -1422,118 +1557,29 @@ static const char *resolve_ref_1(const char *refname,\n \t}\n \n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n-\t\tconst char *path;\n-\t\tstruct stat st;\n-\t\tint fd;\n+\t\tint read_flags = 0;\n \n-\t\tstrbuf_reset(sb_path);\n-\t\tstrbuf_git_path(sb_path, \"%s\", refname);\n-\t\tpath = sb_path->buf;\n-\n-\t\t/*\n-\t\t * We might have to loop back here to avoid a race\n-\t\t * condition: first we lstat() the file, then we try\n-\t\t * to read it as a link or as a file. But if somebody\n-\t\t * changes the type of the file (file <-> directory\n-\t\t * <-> symlink) between the lstat() and reading, then\n-\t\t * we don't want to report that as an error but rather\n-\t\t * try again starting with the lstat().\n-\t\t */\n-\tstat_ref:\n-\t\tif (lstat(path, &st) < 0) {\n-\t\t\tif (errno != ENOENT)\n+\t\tif (read_raw_ref(refname, sha1, sb_refname,\n+\t\t\t\t sb_path, sb_contents, &read_flags)) {\n+\t\t\t*flags |= read_flags;\n+\t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n \t\t\t\treturn NULL;\n-\t\t\tif (resolve_missing_loose_ref(refname, sha1, flags)) {\n-\t\t\t\tif (resolve_flags & RESOLVE_REF_READING) {\n-\t\t\t\t\terrno = ENOENT;\n-\t\t\t\t\treturn NULL;\n-\t\t\t\t}\n-\t\t\t\thashclr(sha1);\n-\t\t\t}\n-\t\t\tif (*flags & REF_BAD_NAME) {\n-\t\t\t\thashclr(sha1);\n+\t\t\thashclr(sha1);\n+\t\t\tif (*flags & REF_BAD_NAME)\n \t\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\t}\n \t\t\treturn refname;\n \t\t}\n \n-\t\t/* Follow \"normalized\" - ie \"refs/..\" symlinks by hand */\n-\t\tif (S_ISLNK(st.st_mode)) {\n-\t\t\tstrbuf_reset(sb_contents);\n-\t\t\tif (strbuf_readlink(sb_contents, path, 0) < 0) {\n-\t\t\t\tif (errno == ENOENT || errno == EINVAL)\n-\t\t\t\t\t/* inconsistent with lstat; retry */\n-\t\t\t\t\tgoto stat_ref;\n-\t\t\t\telse\n-\t\t\t\t\treturn NULL;\n-\t\t\t}\n-\t\t\tif (starts_with(sb_contents->buf, \"refs/\") &&\n-\t\t\t !check_refname_format(sb_contents->buf, 0)) {\n-\t\t\t\tstrbuf_swap(sb_refname, sb_contents);\n-\t\t\t\trefname = sb_refname->buf;\n-\t\t\t\t*flags |= REF_ISSYMREF;\n-\t\t\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n-\t\t\t\t\thashclr(sha1);\n-\t\t\t\t\treturn refname;\n-\t\t\t\t}\n-\t\t\t\tcontinue;\n-\t\t\t}\n-\t\t}\n-\n-\t\t/* Is it a directory? */\n-\t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\terrno = EISDIR;\n-\t\t\treturn NULL;\n-\t\t}\n-\n-\t\t/*\n-\t\t * Anything else, just open it and try to use it as\n-\t\t * a ref\n-\t\t */\n-\t\tfd = open(path, O_RDONLY);\n-\t\tif (fd < 0) {\n-\t\t\tif (errno == ENOENT)\n-\t\t\t\t/* inconsistent with lstat; retry */\n-\t\t\t\tgoto stat_ref;\n-\t\t\telse\n-\t\t\t\treturn NULL;\n-\t\t}\n-\t\tstrbuf_reset(sb_contents);\n-\t\tif (strbuf_read(sb_contents, fd, 256) < 0) {\n-\t\t\tint save_errno = errno;\n-\t\t\tclose(fd);\n-\t\t\terrno = save_errno;\n-\t\t\treturn NULL;\n-\t\t}\n-\t\tclose(fd);\n-\t\tstrbuf_rtrim(sb_contents);\n+\t\t*flags |= read_flags;\n \n-\t\t/*\n-\t\t * Is it a symbolic ref?\n-\t\t */\n-\t\tif (!starts_with(sb_contents->buf, \"ref:\")) {\n-\t\t\t/*\n-\t\t\t * Please note that FETCH_HEAD has a second\n-\t\t\t * line containing other data.\n-\t\t\t */\n-\t\t\tif (get_sha1_hex(sb_contents->buf, sha1) ||\n-\t\t\t (sb_contents->buf[40] != '\\0' && !isspace(sb_contents->buf[40]))) {\n-\t\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\t\terrno = EINVAL;\n-\t\t\t\treturn NULL;\n-\t\t\t}\n+\t\tif (!(read_flags & REF_ISSYMREF)) {\n \t\t\tif (*flags & REF_BAD_NAME) {\n \t\t\t\thashclr(sha1);\n \t\t\t\t*flags |= REF_ISBROKEN;\n \t\t\t}\n \t\t\treturn refname;\n \t\t}\n-\t\t*flags |= REF_ISSYMREF;\n-\t\trefname = sb_contents->buf + 4;\n-\t\twhile (isspace(*refname))\n-\t\t\trefname++;\n-\t\tstrbuf_reset(sb_refname);\n-\t\tstrbuf_addstr(sb_refname, refname);\n+\n \t\trefname = sb_refname->buf;\n \t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n \t\t\thashclr(sha1);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282881","messageId":"1460055791-23313-16-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 15/24] read_raw_ref(): manage own scratch space","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:02Z","receivedAt":"2016-04-07T19:03:02Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nInstead of creating scratch space in resolve_ref_unsafe() and passing it\ndown through resolve_ref_1 to read_raw_ref(), teach read_raw_ref() to\nmanage its own scratch space. This reduces coupling across the functions\nat the cost of some extra allocations. Also, when read_raw_ref() is\nimplemented for different reference backends, the other implementations\nmight have different scratch space requirements.\n\nNote that we now preserve errno across the calls to strbuf_release(),\nwhich calls free() and can thus theoretically overwrite errno.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 76 ++++++++++++++++++++++++++++------------------------\n 1 file changed, 41 insertions(+), 35 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex d51e778..f752568 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1421,17 +1421,20 @@ static int resolve_missing_loose_ref(const char *refname,\n * refname will still be valid and unchanged.\n */\n static int read_raw_ref(const char *refname, unsigned char *sha1,\n-\t\t\tstruct strbuf *symref, struct strbuf *sb_path,\n-\t\t\tstruct strbuf *sb_contents, int *flags)\n+\t\t\tstruct strbuf *symref, int *flags)\n {\n+\tstruct strbuf sb_contents = STRBUF_INIT;\n+\tstruct strbuf sb_path = STRBUF_INIT;\n \tconst char *path;\n \tconst char *buf;\n \tstruct stat st;\n \tint fd;\n+\tint ret = -1;\n+\tint save_errno;\n \n-\tstrbuf_reset(sb_path);\n-\tstrbuf_git_path(sb_path, \"%s\", refname);\n-\tpath = sb_path->buf;\n+\tstrbuf_reset(&sb_path);\n+\tstrbuf_git_path(&sb_path, \"%s\", refname);\n+\tpath = sb_path.buf;\n \n stat_ref:\n \t/*\n@@ -1446,36 +1449,38 @@ stat_ref:\n \n \tif (lstat(path, &st) < 0) {\n \t\tif (errno != ENOENT)\n-\t\t\treturn -1;\n+\t\t\tgoto out;\n \t\tif (resolve_missing_loose_ref(refname, sha1, flags)) {\n \t\t\terrno = ENOENT;\n-\t\t\treturn -1;\n+\t\t\tgoto out;\n \t\t}\n-\t\treturn 0;\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \t/* Follow \"normalized\" - ie \"refs/..\" symlinks by hand */\n \tif (S_ISLNK(st.st_mode)) {\n-\t\tstrbuf_reset(sb_contents);\n-\t\tif (strbuf_readlink(sb_contents, path, 0) < 0) {\n+\t\tstrbuf_reset(&sb_contents);\n+\t\tif (strbuf_readlink(&sb_contents, path, 0) < 0) {\n \t\t\tif (errno == ENOENT || errno == EINVAL)\n \t\t\t\t/* inconsistent with lstat; retry */\n \t\t\t\tgoto stat_ref;\n \t\t\telse\n-\t\t\t\treturn -1;\n+\t\t\t\tgoto out;\n \t\t}\n-\t\tif (starts_with(sb_contents->buf, \"refs/\") &&\n-\t\t !check_refname_format(sb_contents->buf, 0)) {\n-\t\t\tstrbuf_swap(sb_contents, symref);\n+\t\tif (starts_with(sb_contents.buf, \"refs/\") &&\n+\t\t !check_refname_format(sb_contents.buf, 0)) {\n+\t\t\tstrbuf_swap(&sb_contents, symref);\n \t\t\t*flags |= REF_ISSYMREF;\n-\t\t\treturn 0;\n+\t\t\tret = 0;\n+\t\t\tgoto out;\n \t\t}\n \t}\n \n \t/* Is it a directory? */\n \tif (S_ISDIR(st.st_mode)) {\n \t\terrno = EISDIR;\n-\t\treturn -1;\n+\t\tgoto out;\n \t}\n \n \t/*\n@@ -1488,18 +1493,18 @@ stat_ref:\n \t\t\t/* inconsistent with lstat; retry */\n \t\t\tgoto stat_ref;\n \t\telse\n-\t\t\treturn -1;\n+\t\t\tgoto out;\n \t}\n-\tstrbuf_reset(sb_contents);\n-\tif (strbuf_read(sb_contents, fd, 256) < 0) {\n+\tstrbuf_reset(&sb_contents);\n+\tif (strbuf_read(&sb_contents, fd, 256) < 0) {\n \t\tint save_errno = errno;\n \t\tclose(fd);\n \t\terrno = save_errno;\n-\t\treturn -1;\n+\t\tgoto out;\n \t}\n \tclose(fd);\n-\tstrbuf_rtrim(sb_contents);\n-\tbuf = sb_contents->buf;\n+\tstrbuf_rtrim(&sb_contents);\n+\tbuf = sb_contents.buf;\n \tif (starts_with(buf, \"ref:\")) {\n \t\tbuf += 4;\n \t\twhile (isspace(*buf))\n@@ -1508,7 +1513,8 @@ stat_ref:\n \t\tstrbuf_reset(symref);\n \t\tstrbuf_addstr(symref, buf);\n \t\t*flags |= REF_ISSYMREF;\n-\t\treturn 0;\n+\t\tret = 0;\n+\t\tgoto out;\n \t}\n \n \t/*\n@@ -1519,10 +1525,17 @@ stat_ref:\n \t (buf[40] != '\\0' && !isspace(buf[40]))) {\n \t\t*flags |= REF_ISBROKEN;\n \t\terrno = EINVAL;\n-\t\treturn -1;\n+\t\tgoto out;\n \t}\n \n-\treturn 0;\n+\tret = 0;\n+\n+out:\n+\tsave_errno = errno;\n+\tstrbuf_release(&sb_path);\n+\tstrbuf_release(&sb_contents);\n+\terrno = save_errno;\n+\treturn ret;\n }\n \n /* This function needs to return a meaningful errno on failure */\n@@ -1530,9 +1543,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\t\t int resolve_flags,\n \t\t\t\t unsigned char *sha1,\n \t\t\t\t int *flags,\n-\t\t\t\t struct strbuf *sb_refname,\n-\t\t\t\t struct strbuf *sb_path,\n-\t\t\t\t struct strbuf *sb_contents)\n+\t\t\t\t struct strbuf *sb_refname)\n {\n \tint symref_count;\n \n@@ -1559,8 +1570,7 @@ static const char *resolve_ref_1(const char *refname,\n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n \t\tint read_flags = 0;\n \n-\t\tif (read_raw_ref(refname, sha1, sb_refname,\n-\t\t\t\t sb_path, sb_contents, &read_flags)) {\n+\t\tif (read_raw_ref(refname, sha1, sb_refname, &read_flags)) {\n \t\t\t*flags |= read_flags;\n \t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n \t\t\t\treturn NULL;\n@@ -1604,8 +1614,6 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\t\t unsigned char *sha1, int *flags)\n {\n \tstatic struct strbuf sb_refname = STRBUF_INIT;\n-\tstruct strbuf sb_contents = STRBUF_INIT;\n-\tstruct strbuf sb_path = STRBUF_INIT;\n \tint unused_flags;\n \tconst char *ret;\n \n@@ -1613,9 +1621,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t\tflags = &unused_flags;\n \n \tret = resolve_ref_1(refname, resolve_flags, sha1, flags,\n-\t\t\t &sb_refname, &sb_path, &sb_contents);\n-\tstrbuf_release(&sb_path);\n-\tstrbuf_release(&sb_contents);\n+\t\t\t &sb_refname);\n \treturn ret;\n }\n \n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282891","messageId":"1460055791-23313-17-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 16/24] Inline resolve_ref_1() into resolve_ref_unsafe()","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:03Z","receivedAt":"2016-04-07T19:03:03Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nresolve_ref_unsafe() wasn't doing anything useful anymore.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 31 +++++++++----------------------\n 1 file changed, 9 insertions(+), 22 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f752568..120b2dd 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1539,14 +1539,16 @@ out:\n }\n \n /* This function needs to return a meaningful errno on failure */\n-static const char *resolve_ref_1(const char *refname,\n-\t\t\t\t int resolve_flags,\n-\t\t\t\t unsigned char *sha1,\n-\t\t\t\t int *flags,\n-\t\t\t\t struct strbuf *sb_refname)\n+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n+\t\t\t unsigned char *sha1, int *flags)\n {\n+\tstatic struct strbuf sb_refname = STRBUF_INIT;\n+\tint unused_flags;\n \tint symref_count;\n \n+\tif (!flags)\n+\t\tflags = &unused_flags;\n+\n \t*flags = 0;\n \n \tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n@@ -1570,7 +1572,7 @@ static const char *resolve_ref_1(const char *refname,\n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n \t\tint read_flags = 0;\n \n-\t\tif (read_raw_ref(refname, sha1, sb_refname, &read_flags)) {\n+\t\tif (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {\n \t\t\t*flags |= read_flags;\n \t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n \t\t\t\treturn NULL;\n@@ -1590,7 +1592,7 @@ static const char *resolve_ref_1(const char *refname,\n \t\t\treturn refname;\n \t\t}\n \n-\t\trefname = sb_refname->buf;\n+\t\trefname = sb_refname.buf;\n \t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n \t\t\thashclr(sha1);\n \t\t\treturn refname;\n@@ -1610,21 +1612,6 @@ static const char *resolve_ref_1(const char *refname,\n \treturn NULL;\n }\n \n-const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n-\t\t\t unsigned char *sha1, int *flags)\n-{\n-\tstatic struct strbuf sb_refname = STRBUF_INIT;\n-\tint unused_flags;\n-\tconst char *ret;\n-\n-\tif (!flags)\n-\t\tflags = &unused_flags;\n-\n-\tret = resolve_ref_1(refname, resolve_flags, sha1, flags,\n-\t\t\t &sb_refname);\n-\treturn ret;\n-}\n-\n /*\n * Peel the entry (if possible) and return its new peel_status. If\n * repeel is true, re-peel the entry even if there is an old peeled\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282892","messageId":"1460055791-23313-18-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 17/24] read_raw_ref(): change flags parameter to unsigned int","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:04Z","receivedAt":"2016-04-07T19:03:04Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nread_raw_ref() is going to be part of the vtable for reference backends,\nso clean up its interface to use \"unsigned int flags\" rather than \"int\nflags\". Its caller still uses signed int for its flags arguments. But\nchanging that would touch a lot of code, so leave it for now.\n\nSigned-off-by: Michael Haggerty \n---\n refs/files-backend.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 120b2dd..a15986c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1372,7 +1372,7 @@ static struct ref_entry *get_packed_ref(const char *refname)\n */\n static int resolve_missing_loose_ref(const char *refname,\n \t\t\t\t unsigned char *sha1,\n-\t\t\t\t int *flags)\n+\t\t\t\t unsigned int *flags)\n {\n \tstruct ref_entry *entry;\n \n@@ -1421,7 +1421,7 @@ static int resolve_missing_loose_ref(const char *refname,\n * refname will still be valid and unchanged.\n */\n static int read_raw_ref(const char *refname, unsigned char *sha1,\n-\t\t\tstruct strbuf *symref, int *flags)\n+\t\t\tstruct strbuf *symref, unsigned int *flags)\n {\n \tstruct strbuf sb_contents = STRBUF_INIT;\n \tstruct strbuf sb_path = STRBUF_INIT;\n@@ -1570,7 +1570,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n \t}\n \n \tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n-\t\tint read_flags = 0;\n+\t\tunsigned int read_flags = 0;\n \n \t\tif (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {\n \t\t\t*flags |= read_flags;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282890","messageId":"1460055791-23313-19-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 18/24] fsck_head_link(): remove unneeded flag variable","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:05Z","receivedAt":"2016-04-07T19:03:05Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIt is never read, so we can pass NULL to resolve_ref_unsafe().\n\nSigned-off-by: Michael Haggerty \n---\n builtin/fsck.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 55eac75..3f27456 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -493,13 +493,12 @@ static void fsck_object_dir(const char *path)\n \n static int fsck_head_link(void)\n {\n-\tint flag;\n \tint null_is_error = 0;\n \n \tif (verbose)\n \t\tfprintf(stderr, \"Checking HEAD link\\n\");\n \n-\thead_points_at = resolve_ref_unsafe(\"HEAD\", 0, head_oid.hash, &flag);\n+\thead_points_at = resolve_ref_unsafe(\"HEAD\", 0, head_oid.hash, NULL);\n \tif (!head_points_at) {\n \t\terrors_found |= ERROR_REFS;\n \t\treturn error(\"Invalid HEAD\");\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282898","messageId":"1460055791-23313-20-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 19/24] cmd_merge(): remove unneeded flag variable","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:06Z","receivedAt":"2016-04-07T19:03:06Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIt is never read, so we can pass NULL to resolve_ref_unsafe().\n\nSigned-off-by: Michael Haggerty \n---\n builtin/merge.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 101ffef..c90ee51 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1165,7 +1165,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \tstruct commit *head_commit;\n \tstruct strbuf buf = STRBUF_INIT;\n \tconst char *head_arg;\n-\tint flag, i, ret = 0, head_subsumed;\n+\tint i, ret = 0, head_subsumed;\n \tint best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0;\n \tstruct commit_list *common = NULL;\n \tconst char *best_strategy = NULL, *wt_strategy = NULL;\n@@ -1179,7 +1179,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * Check if we are _not_ on a detached HEAD, i.e. if there is a\n \t * current branch.\n \t */\n-\tbranch = branch_to_free = resolve_refdup(\"HEAD\", 0, head_sha1, &flag);\n+\tbranch = branch_to_free = resolve_refdup(\"HEAD\", 0, head_sha1, NULL);\n \tif (branch && starts_with(branch, \"refs/heads/\"))\n \t\tbranch += 11;\n \tif (!branch || is_null_sha1(head_sha1))\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282897","messageId":"1460055791-23313-21-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 20/24] checkout_paths(): remove unneeded flag variable","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:07Z","receivedAt":"2016-04-07T19:03:07Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIt is never read, so we can pass NULL to resolve_ref_unsafe().\n\nSigned-off-by: Michael Haggerty \n---\n builtin/checkout.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex efcbd8f..ea2fe1c 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -242,7 +242,6 @@ static int checkout_paths(const struct checkout_opts *opts,\n \tstruct checkout state;\n \tstatic char *ps_matched;\n \tunsigned char rev[20];\n-\tint flag;\n \tstruct commit *head;\n \tint errs = 0;\n \tstruct lock_file *lock_file;\n@@ -375,7 +374,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \tif (write_locked_index(&the_index, lock_file, COMMIT_LOCK))\n \t\tdie(_(\"unable to write new index file\"));\n \n-\tread_ref_full(\"HEAD\", 0, rev, &flag);\n+\tread_ref_full(\"HEAD\", 0, rev, NULL);\n \thead = lookup_commit_reference_gently(rev, 1);\n \n \terrs |= post_checkout_hook(head, head, 0);\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282886","messageId":"1460055791-23313-22-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 21/24] check_aliased_update(): check that dst_name is non-NULL","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:08Z","receivedAt":"2016-04-07T19:03:08Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nIf there is an error in resolve_ref_unsafe(), it returns NULL. We check\nfor this case, but not until after calling strip_namespace(). Instead,\ncall strip_namespace() *after* the NULL check.\n\nSigned-off-by: Michael Haggerty \n---\n builtin/receive-pack.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex c8e32b2..49cc88d 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1081,13 +1081,13 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)\n \tif (!(flag & REF_ISSYMREF))\n \t\treturn;\n \n-\tdst_name = strip_namespace(dst_name);\n \tif (!dst_name) {\n \t\trp_error(\"refusing update to broken symref '%s'\", cmd->ref_name);\n \t\tcmd->skip_update = 1;\n \t\tcmd->error_string = \"broken symref\";\n \t\treturn;\n \t}\n+\tdst_name = strip_namespace(dst_name);\n \n \tif ((item = string_list_lookup(list, dst_name)) == NULL)\n \t\treturn;\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282887","messageId":"1460055791-23313-23-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 22/24] show_head_ref(): check the result of resolve_ref_namespace()","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:09Z","receivedAt":"2016-04-07T19:03:09Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"From: Michael Haggerty \n\nOnly use the result of resolve_ref_namespace() if it is non-NULL.\n\nSigned-off-by: Michael Haggerty \n---\n http-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/http-backend.c b/http-backend.c\nindex 8870a26..2148814 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -484,9 +484,9 @@ static int show_head_ref(const char *refname, const struct object_id *oid,\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\tconst char *target_nons = strip_namespace(target);\n \n-\t\tstrbuf_addf(buf, \"ref: %s\\n\", target_nons);\n+\t\tif (target)\n+\t\t\tstrbuf_addf(buf, \"ref: %s\\n\", strip_namespace(target));\n \t} else {\n \t\tstrbuf_addf(buf, \"%s\\n\", oid_to_hex(oid));\n \t}\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282889","messageId":"1460055791-23313-24-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 23/24] refs: move resolve_ref_unsafe into common code","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:10Z","receivedAt":"2016-04-07T19:03:10Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Now that resolve_ref_unsafe's only interaction with the backend is\nthrough read_raw_ref, we can move it into the common code. Later,\nwe'll replace read_raw_ref with a backend function.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \n---\n refs.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++\n refs/files-backend.c | 82 ++--------------------------------------------------\n refs/refs-internal.h | 6 ++++\n 3 files changed, 83 insertions(+), 79 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex f0feff7..87dc82f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1155,3 +1155,77 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n \treturn do_for_each_ref(NULL, \"\", fn, 0,\n \t\t\t DO_FOR_EACH_INCLUDE_BROKEN, cb_data);\n }\n+\n+/* This function needs to return a meaningful errno on failure */\n+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n+\t\t\t unsigned char *sha1, int *flags)\n+{\n+\tstatic struct strbuf sb_refname = STRBUF_INIT;\n+\tint unused_flags;\n+\tint symref_count;\n+\n+\tif (!flags)\n+\t\tflags = &unused_flags;\n+\n+\t*flags = 0;\n+\n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n+\t\t !refname_is_safe(refname)) {\n+\t\t\terrno = EINVAL;\n+\t\t\treturn NULL;\n+\t\t}\n+\n+\t\t/*\n+\t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n+\t\t * missing refs and refs that were present but invalid,\n+\t\t * to complain about the latter to stderr.\n+\t\t *\n+\t\t * We don't know whether the ref exists, so don't set\n+\t\t * REF_ISBROKEN yet.\n+\t\t */\n+\t\t*flags |= REF_BAD_NAME;\n+\t}\n+\n+\tfor (symref_count = 0; symref_count < SYMREF_MAXDEPTH; symref_count++) {\n+\t\tunsigned int read_flags = 0;\n+\n+\t\tif (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {\n+\t\t\t*flags |= read_flags;\n+\t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n+\t\t\t\treturn NULL;\n+\t\t\thashclr(sha1);\n+\t\t\tif (*flags & REF_BAD_NAME)\n+\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\treturn refname;\n+\t\t}\n+\n+\t\t*flags |= read_flags;\n+\n+\t\tif (!(read_flags & REF_ISSYMREF)) {\n+\t\t\tif (*flags & REF_BAD_NAME) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\t*flags |= REF_ISBROKEN;\n+\t\t\t}\n+\t\t\treturn refname;\n+\t\t}\n+\n+\t\trefname = sb_refname.buf;\n+\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n+\t\t\thashclr(sha1);\n+\t\t\treturn refname;\n+\t\t}\n+\t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n+\t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n+\t\t\t !refname_is_safe(refname)) {\n+\t\t\t\terrno = EINVAL;\n+\t\t\t\treturn NULL;\n+\t\t\t}\n+\n+\t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n+\t\t}\n+\t}\n+\n+\terrno = ELOOP;\n+\treturn NULL;\n+}\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex a15986c..71848ab 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1269,8 +1269,6 @@ static struct ref_dir *get_loose_refs(struct ref_cache *refs)\n \treturn get_ref_dir(refs->loose);\n }\n \n-/* We allow \"recursive\" symbolic refs. Only within reason, though */\n-#define MAXDEPTH 5\n #define MAXREFLEN (1024)\n \n /*\n@@ -1300,7 +1298,7 @@ static int resolve_gitlink_ref_recursive(struct ref_cache *refs,\n \tchar buffer[128], *p;\n \tchar *path;\n \n-\tif (recursion > MAXDEPTH || strlen(refname) > MAXREFLEN)\n+\tif (recursion > SYMREF_MAXDEPTH || strlen(refname) > MAXREFLEN)\n \t\treturn -1;\n \tpath = *refs->name\n \t\t? git_pathdup_submodule(refs->name, \"%s\", refname)\n@@ -1420,8 +1418,8 @@ static int resolve_missing_loose_ref(const char *refname,\n * - in all other cases, symref will be untouched, and therefore\n * refname will still be valid and unchanged.\n */\n-static int read_raw_ref(const char *refname, unsigned char *sha1,\n-\t\t\tstruct strbuf *symref, unsigned int *flags)\n+int read_raw_ref(const char *refname, unsigned char *sha1,\n+\t\t struct strbuf *symref, unsigned int *flags)\n {\n \tstruct strbuf sb_contents = STRBUF_INIT;\n \tstruct strbuf sb_path = STRBUF_INIT;\n@@ -1538,80 +1536,6 @@ out:\n \treturn ret;\n }\n \n-/* This function needs to return a meaningful errno on failure */\n-const char *resolve_ref_unsafe(const char *refname, int resolve_flags,\n-\t\t\t unsigned char *sha1, int *flags)\n-{\n-\tstatic struct strbuf sb_refname = STRBUF_INIT;\n-\tint unused_flags;\n-\tint symref_count;\n-\n-\tif (!flags)\n-\t\tflags = &unused_flags;\n-\n-\t*flags = 0;\n-\n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t !refname_is_safe(refname)) {\n-\t\t\terrno = EINVAL;\n-\t\t\treturn NULL;\n-\t\t}\n-\n-\t\t/*\n-\t\t * dwim_ref() uses REF_ISBROKEN to distinguish between\n-\t\t * missing refs and refs that were present but invalid,\n-\t\t * to complain about the latter to stderr.\n-\t\t *\n-\t\t * We don't know whether the ref exists, so don't set\n-\t\t * REF_ISBROKEN yet.\n-\t\t */\n-\t\t*flags |= REF_BAD_NAME;\n-\t}\n-\n-\tfor (symref_count = 0; symref_count < MAXDEPTH; symref_count++) {\n-\t\tunsigned int read_flags = 0;\n-\n-\t\tif (read_raw_ref(refname, sha1, &sb_refname, &read_flags)) {\n-\t\t\t*flags |= read_flags;\n-\t\t\tif (errno != ENOENT || (resolve_flags & RESOLVE_REF_READING))\n-\t\t\t\treturn NULL;\n-\t\t\thashclr(sha1);\n-\t\t\tif (*flags & REF_BAD_NAME)\n-\t\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\treturn refname;\n-\t\t}\n-\n-\t\t*flags |= read_flags;\n-\n-\t\tif (!(read_flags & REF_ISSYMREF)) {\n-\t\t\tif (*flags & REF_BAD_NAME) {\n-\t\t\t\thashclr(sha1);\n-\t\t\t\t*flags |= REF_ISBROKEN;\n-\t\t\t}\n-\t\t\treturn refname;\n-\t\t}\n-\n-\t\trefname = sb_refname.buf;\n-\t\tif (resolve_flags & RESOLVE_REF_NO_RECURSE) {\n-\t\t\thashclr(sha1);\n-\t\t\treturn refname;\n-\t\t}\n-\t\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL)) {\n-\t\t\tif (!(resolve_flags & RESOLVE_REF_ALLOW_BAD_NAME) ||\n-\t\t\t !refname_is_safe(refname)) {\n-\t\t\t\terrno = EINVAL;\n-\t\t\t\treturn NULL;\n-\t\t\t}\n-\n-\t\t\t*flags |= REF_ISBROKEN | REF_BAD_NAME;\n-\t\t}\n-\t}\n-\n-\terrno = ELOOP;\n-\treturn NULL;\n-}\n-\n /*\n * Peel the entry (if possible) and return its new peel_status. If\n * repeel is true, re-peel the entry even if there is an old peeled\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 92aae80..3a4f634 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -197,6 +197,8 @@ const char *find_descendant_ref(const char *dirname,\n \n int rename_ref_available(const char *oldname, const char *newname);\n \n+/* We allow \"recursive\" symbolic refs. Only within reason, though */\n+#define SYMREF_MAXDEPTH 5\n \n /* Include broken references in a do_for_each_ref*() iteration: */\n #define DO_FOR_EACH_INCLUDE_BROKEN 0x01\n@@ -206,4 +208,8 @@ int rename_ref_available(const char *oldname, const char *newname);\n */\n int do_for_each_ref(const char *submodule, const char *base,\n \t\t each_ref_fn fn, int trim, int flags, void *cb_data);\n+\n+int read_raw_ref(const char *refname, unsigned char *sha1,\n+\t\t struct strbuf *symref, unsigned int *flags);\n+\n #endif /* REFS_REFS_INTERNAL_H */\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282888","messageId":"1460055791-23313-25-git-send-email-dturner@twopensource.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"[PATCH 24/24] refs: on symref reflog expire, lock symref not referrent","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-07T19:03:11Z","receivedAt":"2016-04-07T19:03:11Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"When locking a symbolic ref to expire a reflog, lock the symbolic\nref (using REF_NODEREF) instead of its referent.\n\nAdd a test for this.\n\nSigned-off-by: David Turner \nSigned-off-by: Junio C Hamano \n---\n refs/files-backend.c | 3 ++-\n t/t1410-reflog.sh | 10 ++++++++++\n 2 files changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 71848ab..5e67bfa 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3314,7 +3314,8 @@ int reflog_expire(const char *refname, const unsigned char *sha1,\n \t * reference itself, plus we might need to update the\n \t * reference if --updateref was specified:\n \t */\n-\tlock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, 0, &type, &err);\n+\tlock = lock_ref_sha1_basic(refname, sha1, NULL, NULL, REF_NODEREF,\n+\t\t\t\t &type, &err);\n \tif (!lock) {\n \t\terror(\"cannot lock ref '%s': %s\", refname, err.buf);\n \t\tstrbuf_release(&err);\ndiff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh\nindex c623824..9cf91dc 100755\n--- a/t/t1410-reflog.sh\n+++ b/t/t1410-reflog.sh\n@@ -338,4 +338,14 @@ test_expect_failure 'reflog with non-commit entries displays all entries' '\n \ttest_line_count = 3 actual\n '\n \n+test_expect_success 'reflog expire operates on symref not referrent' '\n+\tgit branch -l the_symref &&\n+\tgit branch -l referrent &&\n+\tgit update-ref referrent HEAD &&\n+\tgit symbolic-ref refs/heads/the_symref refs/heads/referrent &&\n+\ttest_when_finished \"rm -f .git/refs/heads/referrent.lock\" &&\n+\ttouch .git/refs/heads/referrent.lock &&\n+\tgit reflog expire --expire=all the_symref\n+'\n+\n test_done\n-- \n2.4.2.767.g62658d5-twtrsrc\n"},{"id":"282915","messageId":"xmqqoa9lxdcf.fsf@gitster.mtv.corp.google.com","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 00/24] Yet another pre-refs-backend series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-07T22:29:04Z","receivedAt":"2016-04-07T22:29:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> We now have quite a large number of patches before we even get into\n> the meat of the pluggable refs backend series. So it's worth breaking\n> those out and getting them in before we get into the main series\n> (which Michael Haggerty swants to redesign a bit anyway).\n>\n> This set of patches should be applied on top of\n> jk/check-repository-format.\n>\n> Michael Haggerty has reviewed those of my patches which are in here\n> except maybe:\n> refs: on symref reflog expire, lock symref not referrent\n> This was the one from later in the series that was straightforward to\n> move to before the vtable; the other two were going to be harder to\n> move and can wait until after the vtable.\n>\n> I have reviewed all of Michael's patches.\n\nThanks for working together well.\nWill queue.\n"},{"id":"282961","messageId":"5707CACB.4030906@alum.mit.edu","threadId":"41957","inReplyTo":"1460055791-23313-4-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 03/24] t1430: test the output and error of some commands more carefully","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-04-08T15:14:19Z","receivedAt":"2016-04-08T15:14:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2016 03:02 PM, David Turner wrote:\n> From: Michael Haggerty \n> \n> Signed-off-by: Michael Haggerty \n> [...]\n\nDavid, even though I wrote these patches, I believe you need to sign\nthem off too since you are the one submitting them to the list (it's a\nchain-of-custody sort of thing).\n\nIt's possible Junio would be willing to add the signoffs to the patches\nif you tell him \"I sign off all of the patches in this series\" but I'm\nnot sure.\n\nMichael\n"},{"id":"282979","messageId":"1460143577.5540.11.camel@twopensource.com","threadId":"41957","inReplyTo":"5707CACB.4030906@alum.mit.edu","subject":"Re: [PATCH 03/24] t1430: test the output and error of some commands more carefully","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-08T19:26:17Z","receivedAt":"2016-04-08T19:26:17Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2016-04-08 at 11:14 -0400, Michael Haggerty wrote:\n> On 04/07/2016 03:02 PM, David Turner wrote:\n> > From: Michael Haggerty \n> > \n> > Signed-off-by: Michael Haggerty \n> > [...]\n> \n> David, even though I wrote these patches, I believe you need to sign\n> them off too since you are the one submitting them to the list (it's\n> a\n> chain-of-custody sort of thing).\n> \n> It's possible Junio would be willing to add the signoffs to the\n> patches\n> if you tell him \"I sign off all of the patches in this series\" but\n> I'm\n> not sure.\n\nJunio, I approve signing off on all of these patches. Would you like\nme to re-roll with my sign-off, or would you prefer to add it yourself?\n\nThanks.\n"},{"id":"282995","messageId":"xmqq4mbbvnjk.fsf@gitster.mtv.corp.google.com","threadId":"41957","inReplyTo":"1460143577.5540.11.camel@twopensource.com","subject":"Re: [PATCH 03/24] t1430: test the output and error of some commands more carefully","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-08T20:43:59Z","receivedAt":"2016-04-08T20:43:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner writes:\n\n> Would you like\n> me to re-roll with my sign-off, or would you prefer to add it yourself?\n\nI take it that you see no other reason to reroll the series at this\npoint (otherwise you wouldn't be asking ;-), so I'll check which\nones need amending and add them.\n\nThanks.\n"},{"id":"282996","messageId":"1460152657.5540.16.camel@twopensource.com","threadId":"41957","inReplyTo":"xmqq4mbbvnjk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 03/24] t1430: test the output and error of some commands more carefully","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2016-04-08T21:57:37Z","receivedAt":"2016-04-08T21:57:37Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Fri, 2016-04-08 at 13:43 -0700, Junio C Hamano wrote:\n> David Turner writes:\n> \n> > Would you like\n> > me to re-roll with my sign-off, or would you prefer to add it\n> > yourself?\n> \n> I take it that you see no other reason to reroll the series at this\n> point (otherwise you wouldn't be asking ;-), so I'll check which\n> ones need amending and add them.\n\nThat is correct.\n"},{"id":"283023","messageId":"57092BA6.1060808@alum.mit.edu","threadId":"41957","inReplyTo":"1460055791-23313-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH 00/24] Yet another pre-refs-backend series","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-04-09T16:19:50Z","receivedAt":"2016-04-09T16:19:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/07/2016 03:02 PM, David Turner wrote:\n> We now have quite a large number of patches before we even get into\n> the meat of the pluggable refs backend series. So it's worth breaking\n> those out and getting them in before we get into the main series\n> (which Michael Haggerty swants to redesign a bit anyway).\n> \n> This set of patches should be applied on top of\n> jk/check-repository-format.\n> \n> Michael Haggerty has reviewed those of my patches which are in here\n> except maybe:\n> refs: on symref reflog expire, lock symref not referrent\n> This was the one from later in the series that was straightforward to\n> move to before the vtable; the other two were going to be harder to\n> move and can wait until after the vtable.\n\nThis last patch deserves a little bit of discussion. Currently, when the\nreflog of a symref is expired, the pointed-to ref is locked rather than\nthe symref. This patch changes the code to lock the symref instead.\n\nThis is clearly the right thing to do, and I consider this change a bug\nfix. However, it introduces an incompatibility. An old version of `git\nreflog expire` and a new version wouldn't agree on the locking protocol,\nand could potentially try to overwrite the same reflog at the same time.\n\nI think this risk is acceptable nevertheless, because expiring reflogs\nis an uncommon operation and unlikely to be done from two processes at\nthe same time; moreover, the integrity of reflogs is not a matter of\nlife or death.\n\nA far more likely conflict would be between a reflog expiration and a\nsymref update (e.g., `git checkout otherbranch`). This use case is\ncurrently *broken* because `git checkout` locks HEAD. It would be fixed\nby this patch.\n\nIf somebody is really upset about the risk of a race between an old and\nnew version of `git reflog expire`, the way to increase the safety would\nbe to lock *both* the symref and the referent while changing the\nsymref's reflog. I think that would be overkill.\n\nThis whole series is\n\nReviewed-by: Michael Haggerty \n\nDavid mentioned that I want to redesign the vtable patches somewhat.\nAnybody who is curious can look at the work in progress branch on my\nGitHub fork [1], branch wip/ref-storage.\n\nMichael\n\n[1] https://github.com/mhagger/git\n"},{"id":"283061","messageId":"CAPc5daXvr-doTg482nM164189gjr-M=R8FdDXyVOaduzuepAJA@mail.gmail.com","threadId":"41957","inReplyTo":"57092BA6.1060808@alum.mit.edu","subject":"Re: [PATCH 00/24] Yet another pre-refs-backend series","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-04-10T02:10:06Z","receivedAt":"2016-04-10T02:10:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Sat, Apr 9, 2016 at 9:19 AM, Michael Haggerty wrote:\n>\n> I think this risk is acceptable nevertheless, because expiring reflogs\n> is an uncommon operation and unlikely to be done from two processes at\n> the same time; moreover, the integrity of reflogs is not a matter of\n> life or death.\n> ...\n> If somebody is really upset about the risk of a race between an old and\n> new version of `git reflog expire`, the way to increase the safety would\n> be to lock *both* the symref and the referent while changing the\n> symref's reflog. I think that would be overkill.\n\nThanks, I agree with all of the above.\n\n> This whole series is\n>\n> Reviewed-by: Michael Haggerty \n\nWill squeeze it into all of them, then, and merge to 'next'.\n\nThanks.\n"}]}