Re: [PATCH 1/2] git_mkstemps: correctly test return value of open()
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?
Show changes to 4 files +23 −1
Makefile, t/t0070-fundamental.sh, test-close-fd-0.c, wrapper.c
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
Thomas Rast
