From 32d8b4226a22ef2b4c22cddda7e134980bbabaf6 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:33:51 -0400
Subject: t5613: drop reachable_via function

This function was never used since its inception in dd05ea1
(test case for transitive info/alternates, 2006-05-07).
Which is just as well, since it mutates the repo state in a
way that would invalidate further tests, without cleaning up
after itself. Let's get rid of it so that nobody is tempted
to use it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 9cd2626db..e13f57d26 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -6,16 +6,6 @@
 test_description='test transitive info/alternate entries'
 . ./test-lib.sh
 
-# test that a file is not reachable in the current repository
-# but that it is after creating a info/alternate entry
-reachable_via() {
-	alternate="$1"
-	file="$2"
-	if git cat-file -e "HEAD:$file"; then return 1; fi
-	echo "$alternate" >> .git/objects/info/alternate
-	git cat-file -e "HEAD:$file"
-}
-
 test_valid_repo() {
 	git fsck --full > fsck.log &&
 	test_line_count = 0 fsck.log
-- 
cgit v1.2.1


From b28a88f26a21e6134a099e2d57aa088bc6d93818 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:33:58 -0400
Subject: t5613: drop test_valid_repo function

This function makes sure that "git fsck" does not report any
errors. But "--full" has been the default since f29cd39
(fsck: default to "git fsck --full", 2009-10-20), and we can
use the exit code (instead of counting the lines) since
e2b4f63 (fsck: exit with non-zero status upon errors,
2007-03-05).

So we can just use "git fsck", which is shorter and more
flexible (e.g., we can use "git -C").

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index e13f57d26..4548fb0ab 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -6,11 +6,6 @@
 test_description='test transitive info/alternate entries'
 . ./test-lib.sh
 
-test_valid_repo() {
-	git fsck --full > fsck.log &&
-	test_line_count = 0 fsck.log
-}
-
 base_dir=$(pwd)
 
 test_expect_success 'preparing first repository' \
@@ -52,7 +47,7 @@ git clone --bare -l -s G H'
 
 test_expect_success 'invalidity of deepest repository' \
 'cd H && {
-	test_valid_repo
+	git fsck
 	test $? -ne 0
 }'
 
@@ -60,41 +55,41 @@ cd "$base_dir"
 
 test_expect_success 'validity of third repository' \
 'cd C &&
-test_valid_repo'
+git fsck'
 
 cd "$base_dir"
 
 test_expect_success 'validity of fourth repository' \
 'cd D &&
-test_valid_repo'
+git fsck'
 
 cd "$base_dir"
 
 test_expect_success 'breaking of loops' \
 'echo "$base_dir"/B/.git/objects >> "$base_dir"/A/.git/objects/info/alternates&&
 cd C &&
-test_valid_repo'
+git fsck'
 
 cd "$base_dir"
 
 test_expect_success 'that info/alternates is necessary' \
 'cd C &&
 rm -f .git/objects/info/alternates &&
-! (test_valid_repo)'
+! (git fsck)'
 
 cd "$base_dir"
 
 test_expect_success 'that relative alternate is possible for current dir' \
 'cd C &&
 echo "../../../B/.git/objects" > .git/objects/info/alternates &&
-test_valid_repo'
+git fsck'
 
 cd "$base_dir"
 
 test_expect_success \
     'that relative alternate is only possible for current dir' '
     cd D &&
-    ! (test_valid_repo)
+    ! (git fsck)
 '
 
 cd "$base_dir"
-- 
cgit v1.2.1


From 195eb4654631bf6747649e22b0776c988669d829 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:01 -0400
Subject: t5613: use test_must_fail

Besides being our normal style, this correctly checks for an
error exit() versus signal death.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 4548fb0ab..65074ddc1 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -46,10 +46,9 @@ git clone -l -s F G &&
 git clone --bare -l -s G H'
 
 test_expect_success 'invalidity of deepest repository' \
-'cd H && {
-	git fsck
-	test $? -ne 0
-}'
+'cd H &&
+test_must_fail git fsck
+'
 
 cd "$base_dir"
 
@@ -75,7 +74,8 @@ cd "$base_dir"
 test_expect_success 'that info/alternates is necessary' \
 'cd C &&
 rm -f .git/objects/info/alternates &&
-! (git fsck)'
+test_must_fail git fsck
+'
 
 cd "$base_dir"
 
@@ -89,7 +89,7 @@ cd "$base_dir"
 test_expect_success \
     'that relative alternate is only possible for current dir' '
     cd D &&
-    ! (git fsck)
+    test_must_fail git fsck
 '
 
 cd "$base_dir"
-- 
cgit v1.2.1


From 128a348d049b9c793b89bde4d2ad57feef4e0f26 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:05 -0400
Subject: t5613: whitespace/style cleanups

Our normal test style these days puts the opening quote of
the body on the description line, and indents the body with
a single tab. This ancient test did not follow this.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 114 +++++++++++++++++++++++++---------------------
 1 file changed, 62 insertions(+), 52 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 65074ddc1..1f283a556 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -8,88 +8,98 @@ test_description='test transitive info/alternate entries'
 
 base_dir=$(pwd)
 
-test_expect_success 'preparing first repository' \
-'test_create_repo A && cd A &&
-echo "Hello World" > file1 &&
-git add file1 &&
-git commit -m "Initial commit" file1 &&
-git repack -a -d &&
-git prune'
+test_expect_success 'preparing first repository' '
+	test_create_repo A &&
+	cd A &&
+	echo "Hello World" > file1 &&
+	git add file1 &&
+	git commit -m "Initial commit" file1 &&
+	git repack -a -d &&
+	git prune
+'
 
 cd "$base_dir"
 
-test_expect_success 'preparing second repository' \
-'git clone -l -s A B && cd B &&
-echo "foo bar" > file2 &&
-git add file2 &&
-git commit -m "next commit" file2 &&
-git repack -a -d -l &&
-git prune'
+test_expect_success 'preparing second repository' '
+	git clone -l -s A B &&
+	cd B &&
+	echo "foo bar" > file2 &&
+	git add file2 &&
+	git commit -m "next commit" file2 &&
+	git repack -a -d -l &&
+	git prune
+'
 
 cd "$base_dir"
 
-test_expect_success 'preparing third repository' \
-'git clone -l -s B C && cd C &&
-echo "Goodbye, cruel world" > file3 &&
-git add file3 &&
-git commit -m "one more" file3 &&
-git repack -a -d -l &&
-git prune'
+test_expect_success 'preparing third repository' '
+	git clone -l -s B C &&
+	cd C &&
+	echo "Goodbye, cruel world" > file3 &&
+	git add file3 &&
+	git commit -m "one more" file3 &&
+	git repack -a -d -l &&
+	git prune
+'
 
 cd "$base_dir"
 
-test_expect_success 'creating too deep nesting' \
-'git clone -l -s C D &&
-git clone -l -s D E &&
-git clone -l -s E F &&
-git clone -l -s F G &&
-git clone --bare -l -s G H'
+test_expect_success 'creating too deep nesting' '
+	git clone -l -s C D &&
+	git clone -l -s D E &&
+	git clone -l -s E F &&
+	git clone -l -s F G &&
+	git clone --bare -l -s G H
+'
 
-test_expect_success 'invalidity of deepest repository' \
-'cd H &&
-test_must_fail git fsck
+test_expect_success 'invalidity of deepest repository' '
+	cd H &&
+	test_must_fail git fsck
 '
 
 cd "$base_dir"
 
-test_expect_success 'validity of third repository' \
-'cd C &&
-git fsck'
+test_expect_success 'validity of third repository' '
+	cd C &&
+	git fsck
+'
 
 cd "$base_dir"
 
-test_expect_success 'validity of fourth repository' \
-'cd D &&
-git fsck'
+test_expect_success 'validity of fourth repository' '
+	cd D &&
+	git fsck
+'
 
 cd "$base_dir"
 
-test_expect_success 'breaking of loops' \
-'echo "$base_dir"/B/.git/objects >> "$base_dir"/A/.git/objects/info/alternates&&
-cd C &&
-git fsck'
+test_expect_success 'breaking of loops' '
+	echo "$base_dir"/B/.git/objects >>"$base_dir"/A/.git/objects/info/alternatesi &&
+	cd C &&
+	git fsck
+'
 
 cd "$base_dir"
 
-test_expect_success 'that info/alternates is necessary' \
-'cd C &&
-rm -f .git/objects/info/alternates &&
-test_must_fail git fsck
+test_expect_success 'that info/alternates is necessary' '
+	cd C &&
+	rm -f .git/objects/info/alternates &&
+	test_must_fail git fsck
 '
 
 cd "$base_dir"
 
-test_expect_success 'that relative alternate is possible for current dir' \
-'cd C &&
-echo "../../../B/.git/objects" > .git/objects/info/alternates &&
-git fsck'
+test_expect_success 'that relative alternate is possible for current dir' '
+	cd C &&
+	echo "../../../B/.git/objects" > .git/objects/info/alternates &&
+	git fsck
+'
 
 cd "$base_dir"
 
-test_expect_success \
-    'that relative alternate is only possible for current dir' '
-    cd D &&
-    test_must_fail git fsck
+test_expect_success 'that relative alternate is only possible for current dir' '
+	cd D &&
+	test_must_fail git fsck
 '
 
 cd "$base_dir"
-- 
cgit v1.2.1


From 8f2ed2675358a6e6543f962f6e1b3ec0737e8079 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:08 -0400
Subject: t5613: do not chdir in main process

Our usual style when working with subdirectories is to chdir
inside a subshell or to use "git -C", which means we do not
have to constantly return to the main test directory. Let's
convert this old test, which does not follow that style.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 92 +++++++++++++++++------------------------------
 1 file changed, 33 insertions(+), 59 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 1f283a556..7bc1c3cae 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -6,44 +6,39 @@
 test_description='test transitive info/alternate entries'
 . ./test-lib.sh
 
-base_dir=$(pwd)
-
 test_expect_success 'preparing first repository' '
-	test_create_repo A &&
-	cd A &&
-	echo "Hello World" > file1 &&
-	git add file1 &&
-	git commit -m "Initial commit" file1 &&
-	git repack -a -d &&
-	git prune
+	test_create_repo A && (
+		cd A &&
+		echo "Hello World" > file1 &&
+		git add file1 &&
+		git commit -m "Initial commit" file1 &&
+		git repack -a -d &&
+		git prune
+	)
 '
 
-cd "$base_dir"
-
 test_expect_success 'preparing second repository' '
-	git clone -l -s A B &&
-	cd B &&
-	echo "foo bar" > file2 &&
-	git add file2 &&
-	git commit -m "next commit" file2 &&
-	git repack -a -d -l &&
-	git prune
+	git clone -l -s A B && (
+		cd B &&
+		echo "foo bar" > file2 &&
+		git add file2 &&
+		git commit -m "next commit" file2 &&
+		git repack -a -d -l &&
+		git prune
+	)
 '
 
-cd "$base_dir"
-
 test_expect_success 'preparing third repository' '
-	git clone -l -s B C &&
-	cd C &&
-	echo "Goodbye, cruel world" > file3 &&
-	git add file3 &&
-	git commit -m "one more" file3 &&
-	git repack -a -d -l &&
-	git prune
+	git clone -l -s B C && (
+		cd C &&
+		echo "Goodbye, cruel world" > file3 &&
+		git add file3 &&
+		git commit -m "one more" file3 &&
+		git repack -a -d -l &&
+		git prune
+	)
 '
 
-cd "$base_dir"
-
 test_expect_success 'creating too deep nesting' '
 	git clone -l -s C D &&
 	git clone -l -s D E &&
@@ -53,55 +48,34 @@ test_expect_success 'creating too deep nesting' '
 '
 
 test_expect_success 'invalidity of deepest repository' '
-	cd H &&
-	test_must_fail git fsck
+	test_must_fail git -C H fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'validity of third repository' '
-	cd C &&
-	git fsck
+	git -C C fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'validity of fourth repository' '
-	cd D &&
-	git fsck
+	git -C D fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'breaking of loops' '
-	echo "$base_dir"/B/.git/objects >>"$base_dir"/A/.git/objects/info/alternatesi &&
-	cd C &&
-	git fsck
+	echo "$(pwd)"/B/.git/objects >>A/.git/objects/info/alternates &&
+	git -C C fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'that info/alternates is necessary' '
-	cd C &&
-	rm -f .git/objects/info/alternates &&
-	test_must_fail git fsck
+	rm -f C/.git/objects/info/alternates &&
+	test_must_fail git -C C fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'that relative alternate is possible for current dir' '
-	cd C &&
-	echo "../../../B/.git/objects" > .git/objects/info/alternates &&
+	echo "../../../B/.git/objects" >C/.git/objects/info/alternates &&
 	git fsck
 '
 
-cd "$base_dir"
-
 test_expect_success 'that relative alternate is only possible for current dir' '
-	cd D &&
-	test_must_fail git fsck
+	test_must_fail git -C D fsck
 '
 
-cd "$base_dir"
-
 test_done
-- 
cgit v1.2.1


From f0f86fa9f3e2c0199d492ce7779761f1b5470b65 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:12 -0400
Subject: t5613: clarify "too deep" recursion tests

These tests are just trying to show that we allow recursion
up to a certain depth, but not past it. But the counting is
a bit non-intuitive, and rather than test at the edge of the
breakage, we test "OK" cases in the middle of the chain.
Let's explain what's going on, and explicitly test the
switch between "OK" and "too deep".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5613-info-alternate.sh | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 7bc1c3cae..62170b765 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -39,6 +39,21 @@ test_expect_success 'preparing third repository' '
 	)
 '
 
+# Note: These tests depend on the hard-coded value of 5 as the maximum depth
+# we will follow recursion. We start the depth at 0 and count links, not
+# repositories. This means that in a chain like:
+#
+#   A --> B --> C --> D --> E --> F --> G --> H
+#      0     1     2     3     4     5     6
+#
+# we are OK at "G", but break at "H", even though "H" is actually the 8th
+# repository, not the 6th, which you might expect. Counting the links allows
+# N+1 repositories, and counting from 0 to 5 inclusive allows 6 links.
+#
+# Note also that we must use "--bare -l" to make the link to H. The "-l"
+# ensures we do not do a connectivity check, and the "--bare" makes sure
+# we do not try to checkout the result (which needs objects), either of
+# which would cause the clone to fail.
 test_expect_success 'creating too deep nesting' '
 	git clone -l -s C D &&
 	git clone -l -s D E &&
@@ -47,16 +62,12 @@ test_expect_success 'creating too deep nesting' '
 	git clone --bare -l -s G H
 '
 
-test_expect_success 'invalidity of deepest repository' '
-	test_must_fail git -C H fsck
-'
-
-test_expect_success 'validity of third repository' '
-	git -C C fsck
+test_expect_success 'validity of seventh repository' '
+	git -C G fsck
 '
 
-test_expect_success 'validity of fourth repository' '
-	git -C D fsck
+test_expect_success 'invalidity of eighth repository' '
+	test_must_fail git -C H fsck
 '
 
 test_expect_success 'breaking of loops' '
-- 
cgit v1.2.1


From 670c359da357639f9f9a814ed646b4d854ec5d55 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:17 -0400
Subject: link_alt_odb_entry: handle normalize_path errors

When we add a new alternate to the list, we try to normalize
out any redundant "..", etc. However, we do not look at the
return value of normalize_path_copy(), and will happily
continue with a path that could not be normalized. Worse,
the normalizing process is done in-place, so we are left
with whatever half-finished working state the normalizing
function was in.

Fortunately, this cannot cause us to read past the end of
our buffer, as that working state will always leave the
NUL from the original path in place. And we do tend to
notice problems when we check is_directory() on the path.
But you can see the nonsense that we feed to is_directory
with an entry like:

  this/../../is/../../way/../../too/../../deep/../../to/../../resolve

in your objects/info/alternates, which yields:

  error: object directory
  /to/e/deep/too/way//ects/this/../../is/../../way/../../too/../../deep/../../to/../../resolve
  does not exist; check .git/objects/info/alternates.

We can easily fix this just by checking the return value.
But that makes it hard to generate a good error message,
since we're normalizing in-place and our input value has
been overwritten by cruft.

Instead, let's provide a strbuf helper that does an in-place
normalize, but restores the original contents on error. This
uses a second buffer under the hood, which is slightly less
efficient, but this is not a performance-critical code path.

The strbuf helper can also properly set the "len" parameter
of the strbuf before returning. Just doing:

  normalize_path_copy(buf.buf, buf.buf);

will shorten the string, but leave buf.len at the original
length. That may be confusing to later code which uses the
strbuf.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c | 11 +++++++++--
 strbuf.c    | 20 ++++++++++++++++++++
 strbuf.h    |  8 ++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 94daf31ec..057262d46 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -263,7 +263,12 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 	}
 	strbuf_addstr(&pathbuf, entry);
 
-	normalize_path_copy(pathbuf.buf, pathbuf.buf);
+	if (strbuf_normalize_path(&pathbuf) < 0) {
+		error("unable to normalize alternate object path: %s",
+		      pathbuf.buf);
+		strbuf_release(&pathbuf);
+		return -1;
+	}
 
 	pfxlen = strlen(pathbuf.buf);
 
@@ -335,7 +340,9 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
 	}
 
 	strbuf_add_absolute_path(&objdirbuf, get_object_directory());
-	normalize_path_copy(objdirbuf.buf, objdirbuf.buf);
+	if (strbuf_normalize_path(&objdirbuf) < 0)
+		die("unable to normalize object directory: %s",
+		    objdirbuf.buf);
 
 	alt_copy = xmemdupz(alt, len);
 	string_list_split_in_place(&entries, alt_copy, sep, -1);
diff --git a/strbuf.c b/strbuf.c
index b839be491..8fec6579f 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -870,3 +870,23 @@ void strbuf_stripspace(struct strbuf *sb, int skip_comments)
 
 	strbuf_setlen(sb, j);
 }
+
+int strbuf_normalize_path(struct strbuf *src)
+{
+	struct strbuf dst = STRBUF_INIT;
+
+	strbuf_grow(&dst, src->len);
+	if (normalize_path_copy(dst.buf, src->buf) < 0) {
+		strbuf_release(&dst);
+		return -1;
+	}
+
+	/*
+	 * normalize_path does not tell us the new length, so we have to
+	 * compute it by looking for the new NUL it placed
+	 */
+	strbuf_setlen(&dst, strlen(dst.buf));
+	strbuf_swap(src, &dst);
+	strbuf_release(&dst);
+	return 0;
+}
diff --git a/strbuf.h b/strbuf.h
index ba8d5f1d4..2262b1268 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -443,6 +443,14 @@ extern int strbuf_getcwd(struct strbuf *sb);
  */
 extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);
 
+
+/**
+ * Normalize in-place the path contained in the strbuf. See
+ * normalize_path_copy() for details. If an error occurs, the contents of "sb"
+ * are left untouched, and -1 is returned.
+ */
+extern int strbuf_normalize_path(struct strbuf *sb);
+
 /**
  * Strip whitespace from a buffer. The second parameter controls if
  * comments are considered contents to be removed or not.
-- 
cgit v1.2.1


From 4ea82473aa310de7543141f96c2e6b23ef9fcd4c Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:34:48 -0400
Subject: link_alt_odb_entry: refactor string handling

The string handling in link_alt_odb_entry() is mostly an
artifact of the original version, which took the path as a
ptr/len combo, and did not have a NUL-terminated string
until we created one in the alternate_object_database
struct.  But since 5bdf0a8 (sha1_file: normalize alt_odb
path before comparing and storing, 2011-09-07), the first
thing we do is put the path into a strbuf, which gives us
some easy opportunities for cleanup.

In particular:

  - we call strlen(pathbuf.buf), which is silly; we can look
    at pathbuf.len.

  - even though we have a strbuf, we don't maintain its
    "len" field when chomping extra slashes from the
    end, and instead keep a separate "pfxlen" variable. We
    can fix this and then drop "pfxlen" entirely.

  - we don't check whether the path is usable until after we
    allocate the new struct, making extra cleanup work for
    ourselves. Since we have a NUL-terminated string, we can
    bump the "is it usable" checks higher in the function.
    While we're at it, we can move that logic to its own
    helper, which makes the flow of link_alt_odb_entry()
    easier to follow.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c | 83 +++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 057262d46..3d8c1e88a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -234,6 +234,36 @@ char *sha1_pack_index_name(const unsigned char *sha1)
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 
+/*
+ * Return non-zero iff the path is usable as an alternate object database.
+ */
+static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
+{
+	struct alternate_object_database *alt;
+
+	/* Detect cases where alternate disappeared */
+	if (!is_directory(path->buf)) {
+		error("object directory %s does not exist; "
+		      "check .git/objects/info/alternates.",
+		      path->buf);
+		return 0;
+	}
+
+	/*
+	 * Prevent the common mistake of listing the same
+	 * thing twice, or object directory itself.
+	 */
+	for (alt = alt_odb_list; alt; alt = alt->next) {
+		if (path->len == alt->name - alt->base - 1 &&
+		    !memcmp(path->buf, alt->base, path->len))
+			return 0;
+	}
+	if (!fspathcmp(path->buf, normalized_objdir))
+		return 0;
+
+	return 1;
+}
+
 /*
  * Prepare alternate object database registry.
  *
@@ -253,8 +283,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 	int depth, const char *normalized_objdir)
 {
 	struct alternate_object_database *ent;
-	struct alternate_object_database *alt;
-	size_t pfxlen, entlen;
+	size_t entlen;
 	struct strbuf pathbuf = STRBUF_INIT;
 
 	if (!is_absolute_path(entry) && relative_base) {
@@ -270,47 +299,26 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 		return -1;
 	}
 
-	pfxlen = strlen(pathbuf.buf);
-
 	/*
 	 * The trailing slash after the directory name is given by
 	 * this function at the end. Remove duplicates.
 	 */
-	while (pfxlen && pathbuf.buf[pfxlen-1] == '/')
-		pfxlen -= 1;
-
-	entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
-	ent = xmalloc(st_add(sizeof(*ent), entlen));
-	memcpy(ent->base, pathbuf.buf, pfxlen);
-	strbuf_release(&pathbuf);
-
-	ent->name = ent->base + pfxlen + 1;
-	ent->base[pfxlen + 3] = '/';
-	ent->base[pfxlen] = ent->base[entlen-1] = 0;
+	while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
+		strbuf_setlen(&pathbuf, pathbuf.len - 1);
 
-	/* Detect cases where alternate disappeared */
-	if (!is_directory(ent->base)) {
-		error("object directory %s does not exist; "
-		      "check .git/objects/info/alternates.",
-		      ent->base);
-		free(ent);
+	if (!alt_odb_usable(&pathbuf, normalized_objdir)) {
+		strbuf_release(&pathbuf);
 		return -1;
 	}
 
-	/* Prevent the common mistake of listing the same
-	 * thing twice, or object directory itself.
-	 */
-	for (alt = alt_odb_list; alt; alt = alt->next) {
-		if (pfxlen == alt->name - alt->base - 1 &&
-		    !memcmp(ent->base, alt->base, pfxlen)) {
-			free(ent);
-			return -1;
-		}
-	}
-	if (!fspathcmp(ent->base, normalized_objdir)) {
-		free(ent);
-		return -1;
-	}
+	entlen = st_add(pathbuf.len, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
+	ent = xmalloc(st_add(sizeof(*ent), entlen));
+	memcpy(ent->base, pathbuf.buf, pathbuf.len);
+
+	ent->name = ent->base + pathbuf.len + 1;
+	ent->base[pathbuf.len] = '/';
+	ent->base[pathbuf.len + 3] = '/';
+	ent->base[entlen-1] = 0;
 
 	/* add the alternate entry */
 	*alt_odb_tail = ent;
@@ -318,10 +326,9 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 	ent->next = NULL;
 
 	/* recursively add alternates */
-	read_info_alternates(ent->base, depth + 1);
-
-	ent->base[pfxlen] = '/';
+	read_info_alternates(pathbuf.buf, depth + 1);
 
+	strbuf_release(&pathbuf);
 	return 0;
 }
 
-- 
cgit v1.2.1


From a5b34d21521c015cd41ced4a3fdde57d79891eb3 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:35:03 -0400
Subject: alternates: provide helper for adding to alternates list

The submodule code wants to temporarily add an alternate
object store to our in-memory alt_odb list, but does it
manually. Let's provide a helper so it can reuse the code in
link_alt_odb_entry().

While we're adding our new add_to_alternates_memory(), let's
document add_to_alternates_file(), as the two are related.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h     | 14 +++++++++++++-
 sha1_file.c | 11 +++++++++++
 submodule.c | 23 +----------------------
 3 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/cache.h b/cache.h
index 6fc0e5ae6..258ce6b0f 100644
--- a/cache.h
+++ b/cache.h
@@ -1389,10 +1389,22 @@ extern struct alternate_object_database {
 extern void prepare_alt_odb(void);
 extern void read_info_alternates(const char * relative_base, int depth);
 extern char *compute_alternate_path(const char *path, struct strbuf *err);
-extern void add_to_alternates_file(const char *reference);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
 
+/*
+ * Add the directory to the on-disk alternates file; the new entry will also
+ * take effect in the current process.
+ */
+extern void add_to_alternates_file(const char *dir);
+
+/*
+ * Add the directory to the in-memory list of alternates (along with any
+ * recursive alternates it points to), but do not modify the on-disk alternates
+ * file.
+ */
+extern void add_to_alternates_memory(const char *dir);
+
 struct pack_window {
 	struct pack_window *next;
 	unsigned char *base;
diff --git a/sha1_file.c b/sha1_file.c
index 3d8c1e88a..7a3b74544 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -440,6 +440,17 @@ void add_to_alternates_file(const char *reference)
 	free(alts);
 }
 
+void add_to_alternates_memory(const char *reference)
+{
+	/*
+	 * Make sure alternates are initialized, or else our entry may be
+	 * overwritten when they are.
+	 */
+	prepare_alt_odb();
+
+	link_alt_odb_entries(reference, strlen(reference), '\n', NULL, 0);
+}
+
 /*
  * Compute the exact path an alternate is at and returns it. In case of
  * error NULL is returned and the human readable error is added to `err`
diff --git a/submodule.c b/submodule.c
index 0ef2ff432..8b3274a9d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -123,9 +123,7 @@ void stage_updated_gitmodules(void)
 static int add_submodule_odb(const char *path)
 {
 	struct strbuf objects_directory = STRBUF_INIT;
-	struct alternate_object_database *alt_odb;
 	int ret = 0;
-	size_t alloc;
 
 	ret = strbuf_git_path_submodule(&objects_directory, path, "objects/");
 	if (ret)
@@ -134,26 +132,7 @@ static int add_submodule_odb(const char *path)
 		ret = -1;
 		goto done;
 	}
-	/* avoid adding it twice */
-	prepare_alt_odb();
-	for (alt_odb = alt_odb_list; alt_odb; alt_odb = alt_odb->next)
-		if (alt_odb->name - alt_odb->base == objects_directory.len &&
-				!strncmp(alt_odb->base, objects_directory.buf,
-					objects_directory.len))
-			goto done;
-
-	alloc = st_add(objects_directory.len, 42); /* for "12/345..." sha1 */
-	alt_odb = xmalloc(st_add(sizeof(*alt_odb), alloc));
-	alt_odb->next = alt_odb_list;
-	xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf);
-	alt_odb->name = alt_odb->base + objects_directory.len;
-	alt_odb->name[2] = '/';
-	alt_odb->name[40] = '\0';
-	alt_odb->name[41] = '\0';
-	alt_odb_list = alt_odb;
-
-	/* add possible alternates from the submodule */
-	read_info_alternates(objects_directory.buf, 0);
+	add_to_alternates_memory(objects_directory.buf);
 done:
 	strbuf_release(&objects_directory);
 	return ret;
-- 
cgit v1.2.1


From 7f0fa2c02a6543bdadae3c4a492daae7dbc8c042 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:35:31 -0400
Subject: alternates: provide helper for allocating alternate

Allocating a struct alternate_object_database is tricky, as
we must over-allocate the buffer to provide scratch space,
and then put in particular '/' and NUL markers.

Let's encapsulate this in a function so that the complexity
doesn't leak into callers (and so that we can modify it
later).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h     |  6 ++++++
 sha1_file.c | 28 +++++++++++++++++++---------
 sha1_name.c |  7 +------
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/cache.h b/cache.h
index 258ce6b0f..ece2c7c9b 100644
--- a/cache.h
+++ b/cache.h
@@ -1392,6 +1392,12 @@ extern char *compute_alternate_path(const char *path, struct strbuf *err);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
 
+/*
+ * Allocate a "struct alternate_object_database" but do _not_ actually
+ * add it to the list of alternates.
+ */
+struct alternate_object_database *alloc_alt_odb(const char *dir);
+
 /*
  * Add the directory to the on-disk alternates file; the new entry will also
  * take effect in the current process.
diff --git a/sha1_file.c b/sha1_file.c
index 7a3b74544..636d07ad1 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -283,7 +283,6 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 	int depth, const char *normalized_objdir)
 {
 	struct alternate_object_database *ent;
-	size_t entlen;
 	struct strbuf pathbuf = STRBUF_INIT;
 
 	if (!is_absolute_path(entry) && relative_base) {
@@ -311,14 +310,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
 		return -1;
 	}
 
-	entlen = st_add(pathbuf.len, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
-	ent = xmalloc(st_add(sizeof(*ent), entlen));
-	memcpy(ent->base, pathbuf.buf, pathbuf.len);
-
-	ent->name = ent->base + pathbuf.len + 1;
-	ent->base[pathbuf.len] = '/';
-	ent->base[pathbuf.len + 3] = '/';
-	ent->base[entlen-1] = 0;
+	ent = alloc_alt_odb(pathbuf.buf);
 
 	/* add the alternate entry */
 	*alt_odb_tail = ent;
@@ -395,6 +387,24 @@ void read_info_alternates(const char * relative_base, int depth)
 	munmap(map, mapsz);
 }
 
+struct alternate_object_database *alloc_alt_odb(const char *dir)
+{
+	struct alternate_object_database *ent;
+	size_t dirlen = strlen(dir);
+	size_t entlen;
+
+	entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
+	ent = xmalloc(st_add(sizeof(*ent), entlen));
+	memcpy(ent->base, dir, dirlen);
+
+	ent->name = ent->base + dirlen + 1;
+	ent->base[dirlen] = '/';
+	ent->base[dirlen + 3] = '/';
+	ent->base[entlen-1] = 0;
+
+	return ent;
+}
+
 void add_to_alternates_file(const char *reference)
 {
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
diff --git a/sha1_name.c b/sha1_name.c
index faf873cf7..98152a68b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -86,12 +86,7 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 		 * alt->name/alt->base while iterating over the
 		 * object databases including our own.
 		 */
-		const char *objdir = get_object_directory();
-		size_t objdir_len = strlen(objdir);
-		fakeent = xmalloc(st_add3(sizeof(*fakeent), objdir_len, 43));
-		memcpy(fakeent->base, objdir, objdir_len);
-		fakeent->name = fakeent->base + objdir_len + 1;
-		fakeent->name[-1] = '/';
+		fakeent = alloc_alt_odb(get_object_directory());
 	}
 	fakeent->next = alt_odb_list;
 
-- 
cgit v1.2.1


From 29ec6af2b81894d3236f2aec100323138023ef4d Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:35:43 -0400
Subject: alternates: encapsulate alt->base munging

The alternate_object_database struct holds a path to the
alternate objects, but we also use that buffer as scratch
space for forming loose object filenames. Let's pull that
logic into a helper function so that we can more easily
modify it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 636d07ad1..b284cea8f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -204,6 +204,13 @@ const char *sha1_file_name(const unsigned char *sha1)
 	return buf;
 }
 
+static const char *alt_sha1_path(struct alternate_object_database *alt,
+				 const unsigned char *sha1)
+{
+	fill_sha1_path(alt->name, sha1);
+	return alt->base;
+}
+
 /*
  * Return the name of the pack or index file with the specified sha1
  * in its filename.  *base and *name are scratch space that must be
@@ -601,8 +608,8 @@ static int check_and_freshen_nonlocal(const unsigned char *sha1, int freshen)
 	struct alternate_object_database *alt;
 	prepare_alt_odb();
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		fill_sha1_path(alt->name, sha1);
-		if (check_and_freshen_file(alt->base, freshen))
+		const char *path = alt_sha1_path(alt, sha1);
+		if (check_and_freshen_file(path, freshen))
 			return 1;
 	}
 	return 0;
@@ -1600,8 +1607,8 @@ static int stat_sha1_file(const unsigned char *sha1, struct stat *st)
 	prepare_alt_odb();
 	errno = ENOENT;
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		fill_sha1_path(alt->name, sha1);
-		if (!lstat(alt->base, st))
+		const char *path = alt_sha1_path(alt, sha1);
+		if (!lstat(path, st))
 			return 0;
 	}
 
@@ -1621,8 +1628,8 @@ static int open_sha1_file(const unsigned char *sha1)
 
 	prepare_alt_odb();
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		fill_sha1_path(alt->name, sha1);
-		fd = git_open_noatime(alt->base);
+		const char *path = alt_sha1_path(alt, sha1);
+		fd = git_open_noatime(path);
 		if (fd >= 0)
 			return fd;
 		if (most_interesting_errno == ENOENT)
-- 
cgit v1.2.1


From 597f9134ded20f882e2bf6bca8b0e1f03981b98d Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:35:51 -0400
Subject: alternates: use a separate scratch space

The alternate_object_database struct uses a single buffer
both for storing the path to the alternate, and as a scratch
buffer for forming object names. This is efficient (since
otherwise we'd end up storing the path twice), but it makes
life hard for callers who just want to know the path to the
alternate. They have to remember to stop reading after
"alt->name - alt->base" bytes, and to subtract one for the
trailing '/'.

It would be much simpler if they could simply access a
NUL-terminated path string. We could encapsulate this in a
function which puts a NUL in the scratch buffer and returns
the string, but that opens up questions about the lifetime
of the result. The first time another caller uses the
alternate, the scratch buffer may get other data tacked onto
it.

Let's instead just store the root path separately from the
scratch buffer. There aren't enough alternates being stored
for the duplicated data to matter for performance, and this
keeps things simple and safe for the callers.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fsck.c              | 10 ++--------
 builtin/submodule--helper.c | 11 +++--------
 cache.h                     |  5 ++++-
 sha1_file.c                 | 28 ++++++++++++----------------
 sha1_name.c                 |  3 ++-
 transport.c                 |  4 +---
 6 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 055dfdcf9..f01b81eeb 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -644,14 +644,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 		fsck_object_dir(get_object_directory());
 
 		prepare_alt_odb();
-		for (alt = alt_odb_list; alt; alt = alt->next) {
-			/* directory name, minus trailing slash */
-			size_t namelen = alt->name - alt->base - 1;
-			struct strbuf name = STRBUF_INIT;
-			strbuf_add(&name, alt->base, namelen);
-			fsck_object_dir(name.buf);
-			strbuf_release(&name);
-		}
+		for (alt = alt_odb_list; alt; alt = alt->next)
+			fsck_object_dir(alt->path);
 	}
 
 	if (check_full) {
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e3fdc0aa7..fd72c9044 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -492,20 +492,16 @@ static int add_possible_reference_from_superproject(
 {
 	struct submodule_alternate_setup *sas = sas_cb;
 
-	/* directory name, minus trailing slash */
-	size_t namelen = alt->name - alt->base - 1;
-	struct strbuf name = STRBUF_INIT;
-	strbuf_add(&name, alt->base, namelen);
-
 	/*
 	 * If the alternate object store is another repository, try the
 	 * standard layout with .git/modules/<name>/objects
 	 */
-	if (ends_with(name.buf, ".git/objects")) {
+	if (ends_with(alt->path, ".git/objects")) {
 		char *sm_alternate;
 		struct strbuf sb = STRBUF_INIT;
 		struct strbuf err = STRBUF_INIT;
-		strbuf_add(&sb, name.buf, name.len - strlen("objects"));
+		strbuf_add(&sb, alt->path, strlen(alt->path) - strlen("objects"));
+
 		/*
 		 * We need to end the new path with '/' to mark it as a dir,
 		 * otherwise a submodule name containing '/' will be broken
@@ -533,7 +529,6 @@ static int add_possible_reference_from_superproject(
 		strbuf_release(&sb);
 	}
 
-	strbuf_release(&name);
 	return 0;
 }
 
diff --git a/cache.h b/cache.h
index ece2c7c9b..555ba713d 100644
--- a/cache.h
+++ b/cache.h
@@ -1383,8 +1383,11 @@ extern void remove_scheduled_dirs(void);
 
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
+
 	char *name;
-	char base[FLEX_ARRAY]; /* more */
+	char *scratch;
+
+	char path[FLEX_ARRAY];
 } *alt_odb_list;
 extern void prepare_alt_odb(void);
 extern void read_info_alternates(const char * relative_base, int depth);
diff --git a/sha1_file.c b/sha1_file.c
index b284cea8f..011532a70 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -208,7 +208,7 @@ static const char *alt_sha1_path(struct alternate_object_database *alt,
 				 const unsigned char *sha1)
 {
 	fill_sha1_path(alt->name, sha1);
-	return alt->base;
+	return alt->scratch;
 }
 
 /*
@@ -261,8 +261,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
 	 * thing twice, or object directory itself.
 	 */
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		if (path->len == alt->name - alt->base - 1 &&
-		    !memcmp(path->buf, alt->base, path->len))
+		if (!strcmp(path->buf, alt->path))
 			return 0;
 	}
 	if (!fspathcmp(path->buf, normalized_objdir))
@@ -401,13 +400,14 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
 	size_t entlen;
 
 	entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
-	ent = xmalloc(st_add(sizeof(*ent), entlen));
-	memcpy(ent->base, dir, dirlen);
+	FLEX_ALLOC_STR(ent, path, dir);
+	ent->scratch = xmalloc(entlen);
+	xsnprintf(ent->scratch, entlen, "%s/", dir);
 
-	ent->name = ent->base + dirlen + 1;
-	ent->base[dirlen] = '/';
-	ent->base[dirlen + 3] = '/';
-	ent->base[entlen-1] = 0;
+	ent->name = ent->scratch + dirlen + 1;
+	ent->scratch[dirlen] = '/';
+	ent->scratch[dirlen + 3] = '/';
+	ent->scratch[entlen-1] = 0;
 
 	return ent;
 }
@@ -1485,11 +1485,8 @@ void prepare_packed_git(void)
 		return;
 	prepare_packed_git_one(get_object_directory(), 1);
 	prepare_alt_odb();
-	for (alt = alt_odb_list; alt; alt = alt->next) {
-		alt->name[-1] = 0;
-		prepare_packed_git_one(alt->base, 0);
-		alt->name[-1] = '/';
-	}
+	for (alt = alt_odb_list; alt; alt = alt->next)
+		prepare_packed_git_one(alt->path, 0);
 	rearrange_packed_git();
 	prepare_packed_git_mru();
 	prepare_packed_git_run_once = 1;
@@ -3692,8 +3689,7 @@ static int loose_from_alt_odb(struct alternate_object_database *alt,
 	struct strbuf buf = STRBUF_INIT;
 	int r;
 
-	/* copy base not including trailing '/' */
-	strbuf_add(&buf, alt->base, alt->name - alt->base - 1);
+	strbuf_addstr(&buf, alt->path);
 	r = for_each_loose_file_in_objdir_buf(&buf,
 					      data->cb, NULL, NULL,
 					      data->data);
diff --git a/sha1_name.c b/sha1_name.c
index 98152a68b..770ea4fe8 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -94,12 +94,13 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 	for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
 		struct dirent *de;
 		DIR *dir;
+
 		/*
 		 * every alt_odb struct has 42 extra bytes after the base
 		 * for exactly this purpose
 		 */
 		xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
-		dir = opendir(alt->base);
+		dir = opendir(alt->scratch);
 		if (!dir)
 			continue;
 
diff --git a/transport.c b/transport.c
index 94d6dc372..4bc4eeae5 100644
--- a/transport.c
+++ b/transport.c
@@ -1084,9 +1084,7 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,
 	const struct ref *extra;
 	struct alternate_refs_data *cb = data;
 
-	e->name[-1] = '\0';
-	other = xstrdup(real_path(e->base));
-	e->name[-1] = '/';
+	other = xstrdup(real_path(e->path));
 	len = strlen(other);
 
 	while (other[len-1] == '/')
-- 
cgit v1.2.1


From afbba2f09ab06424d8b38908f4d76a1503f49a25 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:35:55 -0400
Subject: fill_sha1_file: write "boring" characters

This function forms a sha1 as "xx/yyyy...", but skips over
the slot for the slash rather than writing it, leaving it to
the caller to do so. It also does not bother to put in a
trailing NUL, even though every caller would want it (we're
forming a path which by definition is not a directory, so
the only thing to do with it is feed it to a system call).

Let's make the lives of our callers easier by just writing
out the internal "/" and the NUL.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 011532a70..da7b92260 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -178,10 +178,12 @@ static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
 	for (i = 0; i < 20; i++) {
 		static char hex[] = "0123456789abcdef";
 		unsigned int val = sha1[i];
-		char *pos = pathbuf + i*2 + (i > 0);
-		*pos++ = hex[val >> 4];
-		*pos = hex[val & 0xf];
+		*pathbuf++ = hex[val >> 4];
+		*pathbuf++ = hex[val & 0xf];
+		if (!i)
+			*pathbuf++ = '/';
 	}
+	*pathbuf = '\0';
 }
 
 const char *sha1_file_name(const unsigned char *sha1)
@@ -198,8 +200,6 @@ const char *sha1_file_name(const unsigned char *sha1)
 		die("insanely long object directory %s", objdir);
 	memcpy(buf, objdir, len);
 	buf[len] = '/';
-	buf[len+3] = '/';
-	buf[len+42] = '\0';
 	fill_sha1_path(buf + len + 1, sha1);
 	return buf;
 }
@@ -406,8 +406,6 @@ struct alternate_object_database *alloc_alt_odb(const char *dir)
 
 	ent->name = ent->scratch + dirlen + 1;
 	ent->scratch[dirlen] = '/';
-	ent->scratch[dirlen + 3] = '/';
-	ent->scratch[entlen-1] = 0;
 
 	return ent;
 }
-- 
cgit v1.2.1


From 38dbe5f07837afceaec95fae5981d36eeb4917bd Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:36:04 -0400
Subject: alternates: store scratch buffer as strbuf

We pre-size the scratch buffer to hold a loose object
filename of the form "xx/yyyy...", which leads to allocation
code that is hard to verify. We have to use some magic
numbers during the initial allocation, and then writers must
blindly assume that the buffer is big enough. Using a strbuf
makes it more clear that we cannot overflow.

Unfortunately, we do still need some magic numbers to grow
our strbuf before calling fill_sha1_path(), but the strbuf
growth is much closer to the point of use. This makes it
easier to see that it's correct, and opens the possibility
of pushing it even further down if fill_sha1_path() learns
to work on strbufs.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 cache.h     | 13 +++++++++++--
 sha1_file.c | 28 ++++++++++++++++++----------
 sha1_name.c |  9 +++------
 3 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/cache.h b/cache.h
index 555ba713d..5d36ffa1f 100644
--- a/cache.h
+++ b/cache.h
@@ -1384,8 +1384,9 @@ extern void remove_scheduled_dirs(void);
 extern struct alternate_object_database {
 	struct alternate_object_database *next;
 
-	char *name;
-	char *scratch;
+	/* see alt_scratch_buf() */
+	struct strbuf scratch;
+	size_t base_len;
 
 	char path[FLEX_ARRAY];
 } *alt_odb_list;
@@ -1414,6 +1415,14 @@ extern void add_to_alternates_file(const char *dir);
  */
 extern void add_to_alternates_memory(const char *dir);
 
+/*
+ * Returns a scratch strbuf pre-filled with the alternate object directory,
+ * including a trailing slash, which can be used to access paths in the
+ * alternate. Always use this over direct access to alt->scratch, as it
+ * cleans up any previous use of the scratch buffer.
+ */
+extern struct strbuf *alt_scratch_buf(struct alternate_object_database *alt);
+
 struct pack_window {
 	struct pack_window *next;
 	unsigned char *base;
diff --git a/sha1_file.c b/sha1_file.c
index da7b92260..51d40241a 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -204,11 +204,24 @@ const char *sha1_file_name(const unsigned char *sha1)
 	return buf;
 }
 
+struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
+{
+	strbuf_setlen(&alt->scratch, alt->base_len);
+	return &alt->scratch;
+}
+
 static const char *alt_sha1_path(struct alternate_object_database *alt,
 				 const unsigned char *sha1)
 {
-	fill_sha1_path(alt->name, sha1);
-	return alt->scratch;
+	/* hex sha1 plus internal "/" */
+	size_t len = GIT_SHA1_HEXSZ + 1;
+	struct strbuf *buf = alt_scratch_buf(alt);
+
+	strbuf_grow(buf, len);
+	fill_sha1_path(buf->buf + buf->len, sha1);
+	strbuf_setlen(buf, buf->len + len);
+
+	return buf->buf;
 }
 
 /*
@@ -396,16 +409,11 @@ void read_info_alternates(const char * relative_base, int depth)
 struct alternate_object_database *alloc_alt_odb(const char *dir)
 {
 	struct alternate_object_database *ent;
-	size_t dirlen = strlen(dir);
-	size_t entlen;
 
-	entlen = st_add(dirlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
 	FLEX_ALLOC_STR(ent, path, dir);
-	ent->scratch = xmalloc(entlen);
-	xsnprintf(ent->scratch, entlen, "%s/", dir);
-
-	ent->name = ent->scratch + dirlen + 1;
-	ent->scratch[dirlen] = '/';
+	strbuf_init(&ent->scratch, 0);
+	strbuf_addf(&ent->scratch, "%s/", dir);
+	ent->base_len = ent->scratch.len;
 
 	return ent;
 }
diff --git a/sha1_name.c b/sha1_name.c
index 770ea4fe8..defbb3eb0 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -92,15 +92,12 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
 
 	xsnprintf(hex, sizeof(hex), "%.2s", hex_pfx);
 	for (alt = fakeent; alt && !ds->ambiguous; alt = alt->next) {
+		struct strbuf *buf = alt_scratch_buf(alt);
 		struct dirent *de;
 		DIR *dir;
 
-		/*
-		 * every alt_odb struct has 42 extra bytes after the base
-		 * for exactly this purpose
-		 */
-		xsnprintf(alt->name, 42, "%.2s/", hex_pfx);
-		dir = opendir(alt->scratch);
+		strbuf_addf(buf, "%.2s/", hex_pfx);
+		dir = opendir(buf->buf);
 		if (!dir)
 			continue;
 
-- 
cgit v1.2.1


From f7b7774f34b86fed8a2e9554a9fe865c62a0a5eb Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:36:09 -0400
Subject: fill_sha1_file: write into a strbuf

It's currently the responsibility of the caller to give
fill_sha1_file() enough bytes to write into, leading them to
manually compute the required lengths. Instead, let's just
write into a strbuf so that it's impossible to get this
wrong.

The alt_odb caller already has a strbuf, so this makes
things strictly simpler. The other caller, sha1_file_name(),
uses a static PATH_MAX buffer and dies when it would
overflow. We can convert this to a static strbuf, which
means our allocation cost is amortized (and as a bonus, we
no longer have to worry about PATH_MAX being too short for
normal use).

This does introduce some small overhead in fill_sha1_file(),
as each strbuf_addchar() will check whether it needs to
grow. However, between the optimization in fec501d
(strbuf_addch: avoid calling strbuf_grow, 2015-04-16) and
the fact that this is not generally called in a tight loop
(after all, the next step is typically to access the file!)
this probably doesn't matter. And even if it did, the right
place to micro-optimize is inside fill_sha1_file(), by
calling a single strbuf_grow() there.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 51d40241a..1b86a3ae7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -172,36 +172,28 @@ enum scld_error safe_create_leading_directories_const(const char *path)
 	return result;
 }
 
-static void fill_sha1_path(char *pathbuf, const unsigned char *sha1)
+static void fill_sha1_path(struct strbuf *buf, const unsigned char *sha1)
 {
 	int i;
 	for (i = 0; i < 20; i++) {
 		static char hex[] = "0123456789abcdef";
 		unsigned int val = sha1[i];
-		*pathbuf++ = hex[val >> 4];
-		*pathbuf++ = hex[val & 0xf];
+		strbuf_addch(buf, hex[val >> 4]);
+		strbuf_addch(buf, hex[val & 0xf]);
 		if (!i)
-			*pathbuf++ = '/';
+			strbuf_addch(buf, '/');
 	}
-	*pathbuf = '\0';
 }
 
 const char *sha1_file_name(const unsigned char *sha1)
 {
-	static char buf[PATH_MAX];
-	const char *objdir;
-	int len;
+	static struct strbuf buf = STRBUF_INIT;
 
-	objdir = get_object_directory();
-	len = strlen(objdir);
+	strbuf_reset(&buf);
+	strbuf_addf(&buf, "%s/", get_object_directory());
 
-	/* '/' + sha1(2) + '/' + sha1(38) + '\0' */
-	if (len + 43 > PATH_MAX)
-		die("insanely long object directory %s", objdir);
-	memcpy(buf, objdir, len);
-	buf[len] = '/';
-	fill_sha1_path(buf + len + 1, sha1);
-	return buf;
+	fill_sha1_path(&buf, sha1);
+	return buf.buf;
 }
 
 struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
@@ -213,14 +205,8 @@ struct strbuf *alt_scratch_buf(struct alternate_object_database *alt)
 static const char *alt_sha1_path(struct alternate_object_database *alt,
 				 const unsigned char *sha1)
 {
-	/* hex sha1 plus internal "/" */
-	size_t len = GIT_SHA1_HEXSZ + 1;
 	struct strbuf *buf = alt_scratch_buf(alt);
-
-	strbuf_grow(buf, len);
-	fill_sha1_path(buf->buf + buf->len, sha1);
-	strbuf_setlen(buf, buf->len + len);
-
+	fill_sha1_path(buf, sha1);
 	return buf->buf;
 }
 
-- 
cgit v1.2.1


From 5fe849d651e259af58f29f9cfb1b1405154ffacc Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:36:18 -0400
Subject: count-objects: report alternates via verbose mode

There's no way to get the list of alternates that git
computes internally; our tests only infer it based on which
objects are available. In addition to testing, knowing this
list may be helpful for somebody debugging their alternates
setup.

Let's add it to the "count-objects -v" output. We could give
it a separate flag, but there's not really any need.
"count-objects -v" is already a debugging catch-all for the
object database, its output is easily extensible to new data
items, and printing the alternates is not expensive (we
already had to find them to count the objects).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-count-objects.txt |  5 +++++
 builtin/count-objects.c             | 10 ++++++++++
 t/t5613-info-alternate.sh           | 10 ++++++++++
 3 files changed, 25 insertions(+)

diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
index 2ff35683e..cb9b4d2e4 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -38,6 +38,11 @@ objects nor valid packs
 +
 size-garbage: disk space consumed by garbage files, in KiB (unless -H is
 specified)
++
+alternate: absolute path of alternate object databases; may appear
+multiple times, one line per path. Note that if the path contains
+non-printable characters, it may be surrounded by double-quotes and
+contain C-style backslashed escape sequences.
 
 -H::
 --human-readable::
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index ba9291944..a700409bf 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -8,6 +8,7 @@
 #include "dir.h"
 #include "builtin.h"
 #include "parse-options.h"
+#include "quote.h"
 
 static unsigned long garbage;
 static off_t size_garbage;
@@ -73,6 +74,14 @@ static int count_cruft(const char *basename, const char *path, void *data)
 	return 0;
 }
 
+static int print_alternate(struct alternate_object_database *alt, void *data)
+{
+	printf("alternate: ");
+	quote_c_style(alt->path, NULL, stdout, 0);
+	putchar('\n');
+	return 0;
+}
+
 static char const * const count_objects_usage[] = {
 	N_("git count-objects [-v] [-H | --human-readable]"),
 	NULL
@@ -140,6 +149,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		printf("prune-packable: %lu\n", packed_loose);
 		printf("garbage: %lu\n", garbage);
 		printf("size-garbage: %s\n", garbage_buf.buf);
+		foreach_alt_odb(print_alternate, NULL);
 		strbuf_release(&loose_buf);
 		strbuf_release(&pack_buf);
 		strbuf_release(&garbage_buf);
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 62170b765..e146a8def 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -39,6 +39,16 @@ test_expect_success 'preparing third repository' '
 	)
 '
 
+test_expect_success 'count-objects shows the alternates' '
+	cat >expect <<-EOF &&
+	alternate: $(pwd)/B/.git/objects
+	alternate: $(pwd)/A/.git/objects
+	EOF
+	git -C C count-objects -v >actual &&
+	grep ^alternate: actual >actual.alternates &&
+	test_cmp expect actual.alternates
+'
+
 # Note: These tests depend on the hard-coded value of 5 as the maximum depth
 # we will follow recursion. We start the depth at 0 and count links, not
 # repositories. This means that in a chain like:
-- 
cgit v1.2.1


From 087b6d584062f5b704356286d6445bcc84d686fb Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:36:22 -0400
Subject: sha1_file: always allow relative paths to alternates

We recursively expand alternates repositories, so that if A
borrows from B which borrows from C, A can see all objects.

For the root object database, we allow relative paths, so A
can point to B as "../B/objects". However, we currently do
not allow relative paths when recursing, so B must use an
absolute path to reach C.

That is an ancient protection from c2f493a (Transitively
read alternatives, 2006-05-07) that tries to avoid adding
the same alternate through two different paths. Since
5bdf0a8 (sha1_file: normalize alt_odb path before comparing
and storing, 2011-09-07), we use a normalized absolute path
for each alt_odb entry.

This means that in most cases the protection is no longer
necessary; we will detect the duplicate no matter how we got
there (but see below).  And it's a good idea to get rid of
it, as it creates an unnecessary complication when setting
up recursive alternates (B has to know that A is going to
borrow from it and make sure to use an absolute path).

Note that our normalization doesn't actually look at the
filesystem, so it can still be fooled by crossing symbolic
links. But that's also true of absolute paths, so it's not a
good reason to disallow only relative paths (it's
potentially a reason to switch to real_path(), but that's a
separate and non-trivial change).

We adjust the test script here to demonstrate that this now
works, and add new tests to show that the normalization does
indeed suppress duplicates.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 sha1_file.c               |  7 +------
 t/t5613-info-alternate.sh | 24 ++++++++++++++++++++++--
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 1b86a3ae7..9cad56f7b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -354,12 +354,7 @@ static void link_alt_odb_entries(const char *alt, int len, int sep,
 		const char *entry = entries.items[i].string;
 		if (entry[0] == '\0' || entry[0] == '#')
 			continue;
-		if (!is_absolute_path(entry) && depth) {
-			error("%s: ignoring relative alternate object store %s",
-					relative_base, entry);
-		} else {
-			link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
-		}
+		link_alt_odb_entry(entry, relative_base, depth, objdirbuf.buf);
 	}
 	string_list_clear(&entries, 0);
 	free(alt_copy);
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index e146a8def..76f1a20e2 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -95,8 +95,28 @@ test_expect_success 'that relative alternate is possible for current dir' '
 	git fsck
 '
 
-test_expect_success 'that relative alternate is only possible for current dir' '
-	test_must_fail git -C D fsck
+test_expect_success 'that relative alternate is recursive' '
+	git -C D fsck
+'
+
+# we can reach "A" from our new repo both directly, and via "C".
+# The deep/subdir is there to make sure we are not doing a stupid
+# pure-text comparison of the alternate names.
+test_expect_success 'relative duplicates are eliminated' '
+	mkdir -p deep/subdir &&
+	git init --bare deep/subdir/duplicate.git &&
+	cat >deep/subdir/duplicate.git/objects/info/alternates <<-\EOF &&
+	../../../../C/.git/objects
+	../../../../A/.git/objects
+	EOF
+	cat >expect <<-EOF &&
+	alternate: $(pwd)/C/.git/objects
+	alternate: $(pwd)/B/.git/objects
+	alternate: $(pwd)/A/.git/objects
+	EOF
+	git -C deep/subdir/duplicate.git count-objects -v >actual &&
+	grep ^alternate: actual >actual.alternates &&
+	test_cmp expect actual.alternates
 '
 
 test_done
-- 
cgit v1.2.1


From ea0fc3b4176a424a2b20eb76a6a503dc4d59cebb Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 3 Oct 2016 16:36:26 -0400
Subject: alternates: use fspathcmp to detect duplicates

On a case-insensitive filesystem, we should realize that
"a/objects" and "A/objects" are the same path. We already
use fspathcmp() to check against the main object directory,
but until recently we couldn't use it for comparing against
other alternates (because their paths were not
NUL-terminated strings). But now we can, so let's do so.

Note that we also need to adjust count-objects to load the
config, so that it can see the setting of core.ignorecase
(this is required by the test, but is also a general bugfix
for users of count-objects).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/count-objects.c   |  2 ++
 sha1_file.c               |  2 +-
 t/t5613-info-alternate.sh | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a700409bf..a04b4f2ef 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -97,6 +97,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)
 		OPT_END(),
 	};
 
+	git_config(git_default_config, NULL);
+
 	argc = parse_options(argc, argv, prefix, opts, count_objects_usage, 0);
 	/* we do not take arguments other than flags for now */
 	if (argc)
diff --git a/sha1_file.c b/sha1_file.c
index 9cad56f7b..064651947 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -260,7 +260,7 @@ static int alt_odb_usable(struct strbuf *path, const char *normalized_objdir)
 	 * thing twice, or object directory itself.
 	 */
 	for (alt = alt_odb_list; alt; alt = alt->next) {
-		if (!strcmp(path->buf, alt->path))
+		if (!fspathcmp(path->buf, alt->path))
 			return 0;
 	}
 	if (!fspathcmp(path->buf, normalized_objdir))
diff --git a/t/t5613-info-alternate.sh b/t/t5613-info-alternate.sh
index 76f1a20e2..895f46bb9 100755
--- a/t/t5613-info-alternate.sh
+++ b/t/t5613-info-alternate.sh
@@ -119,4 +119,21 @@ test_expect_success 'relative duplicates are eliminated' '
 	test_cmp expect actual.alternates
 '
 
+test_expect_success CASE_INSENSITIVE_FS 'dup finding can be case-insensitive' '
+	git init --bare insensitive.git &&
+	# the previous entry for "A" will have used uppercase
+	cat >insensitive.git/objects/info/alternates <<-\EOF &&
+	../../C/.git/objects
+	../../a/.git/objects
+	EOF
+	cat >expect <<-EOF &&
+	alternate: $(pwd)/C/.git/objects
+	alternate: $(pwd)/B/.git/objects
+	alternate: $(pwd)/A/.git/objects
+	EOF
+	git -C insensitive.git count-objects -v >actual &&
+	grep ^alternate: actual >actual.alternates &&
+	test_cmp expect actual.alternates
+'
+
 test_done
-- 
cgit v1.2.1