Re: [PATCH] More test cases for sanitized path names

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> I haven't looked at the code, but I suspect that "git add" and
> anything that uses the same logic as "ls-files --error-unmatch"
> would still not work with the setup patch.

Ok, I looked at the code.

I already had a fix for "ls-files --error-unmatch" in the
weatherbaloon patch.  The logic to complain and fail nonsense
paths was needed for "add".

The attached is on top of the previous one and your test case
updates.  We may want to also add tests for --error-unmatch.

It is very tempting to enhance pathspec API in such a way that
it is not just a NULL terminated array of (char*) pointers, but
a pointer to a richer structure that knows the number of
elements and which strings need to be freed (and we would invent
a "free_pathspec()" function to free them) and such, but that is
an independent surgery that is outside of the scope of flying
weatherbaloons.

---

 builtin-add.c    |   12 ++++++++++++
 t/t7010-setup.sh |   20 ++++++++++++++------
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/builtin-add.c b/builtin-add.c
index 4a91e3e..820110e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -228,6 +228,18 @@ int cmd_add(int argc, const char **argv, const char *prefix)
 		goto finish;
 	}
 
+	if (*argv) {
+		/* Was there an invalid path? */
+		if (pathspec) {
+			int num;
+			for (num = 0; pathspec[num]; num++)
+				; /* just counting */
+			if (argc != num)
+				exit(1); /* error message already given */
+		} else
+			exit(1); /* error message already given */
+	}
+
 	fill_directory(&dir, pathspec, ignored_too);
 
 	if (show_only) {
diff --git a/t/t7010-setup.sh b/t/t7010-setup.sh
index 60c4a46..e809e0e 100755
--- a/t/t7010-setup.sh
+++ b/t/t7010-setup.sh
@@ -135,20 +135,28 @@ test_expect_success 'blame using absolute path names' '
 	diff -u f1.txt f2.txt
 '
 
-test_expect_failure 'add a directory outside the work tree' '
+test_expect_success 'setup deeper work tree' '
+	test_create_repo tester
+'
+
+test_expect_success 'add a directory outside the work tree' '(
+	cd tester &&
 	d1="$(cd .. ; pwd)" &&
 	git add "$d1"
-	echo $?
-'
+)'
 
-test_expect_failure 'add a file outside the work tree, nasty case 1' '(
+test_expect_success 'add a file outside the work tree, nasty case 1' '(
+	cd tester &&
 	f="$(pwd)x" &&
+	echo "$f" &&
 	touch "$f" &&
 	git add "$f"
 )'
 
-test_expect_failure 'add a file outside the work tree, nasty case 2' '(
-	f="$(pwd|sed "s/.$//")x" &&
+test_expect_success 'add a file outside the work tree, nasty case 2' '(
+	cd tester &&
+	f="$(pwd | sed "s/.$//")x" &&
+	echo "$f" &&
 	touch "$f" &&
 	git add "$f"
 )'