Re: [PATCH 1/2] git_mkstemps: correctly test return value of open()
- From
Dale R. Worley <worley@alum.mit.edu>
- Date
- Jul 18, 2013, 20:32 UTC
- Message-ID
- <201307182032.r6IKWtWC016218@freeze.ariadne.com>
- In-Reply-To
- <7v4nbr4v7m.fsf@alter.siamese.dyndns.org>
I've been looking into writing a proper test for this patch. My first attempt tests the symptom that was seen initially, that "git commit" fails if fd 0 is closed.
One problem is how to arrange for fd 0 to be closed. I could use the bash redirection "<&-", but I think you want to be more portable than that. This version uses execvp() inside a small C program, and execvp() is a Posix function.
I've tested that this test does what it should: If you remove the fix, "fd >= 0", the test fails. If you then remove "test-close-fd-0" from before "git init" in the test, the test is nullified and succeeds again.
Here is the diff. What do people think of it?
diff --git a/Makefile b/Makefile index 0600eb4..6b410f5 100644 --- a/Makefile +++ b/Makefile @@ -557,6 +557,7 @@ X = PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS)) TEST_PROGRAMS_NEED_X += test-chmtime +TEST_PROGRAMS_NEED_X += test-close-fd-0 TEST_PROGRAMS_NEED_X += test-ctype TEST_PROGRAMS_NEED_X += test-date TEST_PROGRAMS_NEED_X += test-delta diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh index 986b2a8..6a31103 100755 --- a/t/t0070-fundamental.sh +++ b/t/t0070-fundamental.sh @@ -25,6 +25,13 @@ test_expect_success POSIXPERM,SANITY 'mktemp to unwritable directory prints file grep "cannotwrite/test" err ' +test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' ' + git init && + echo Test. >test-file && + git add test-file && + test-close-fd-0 git commit -m Message. +' + test_expect_success 'check for a bug in the regex routines' ' # if this test fails, re-build git with NO_REGEX=1 test-regex diff --git a/test-close-fd-0.c b/test-close-fd-0.c new file mode 100644 index 0000000..3745c34 --- /dev/null +++ b/test-close-fd-0.c @@ -0,0 +1,14 @@ +#include <unistd.h> + +/* Close file descriptor 0 (which is standard-input), then execute the + * remainder of the command line as a command. */ + +int main(int argc, char **argv) +{ + /* Close fd 0. */ + close(0); + /* Execute the requested command. */ + execvp(argv[1], &argv[1]); + /* If execve() failed, return an error. */ + return 1; +} diff --git a/wrapper.c b/wrapper.c index dd7ecbb..6a015de 100644 --- a/wrapper.c +++ b/wrapper.c @@ -322,7 +322,7 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode) template[5] = letters[v % num_letters]; v /= num_letters; fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode); - if (fd > 0) + if (fd >= 0) return fd; /* * Fatal error (EPERM, ENOSPC etc). Dale