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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to