Re: [PATCH] More test cases for sanitized path names
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 1, 2008, 11:10 UTC
- Message-ID
- <7vtzkt7x02.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <7v1w7x9cgf.fsf@gitster.siamese.dyndns.org>
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" )'