[PATCH] git_mkstemps: improve test suite test

[PATCH] git_mkstemps: improve test suite test

[Dale R. Worley]

Commit 52749 fixes a bug regarding testing the return of an open()
call for success/failure.  Improve the testsuite test for that fix by
removing the helper program 'test-close-fd-0' and replacing it with
the shell redirection '<&-'.  (The redirection is Posix, so it should
be portable.)

Signed-off-by: Dale Worley <worley@ariadne.com>
---

> From: Junio C Hamano <gitster@pobox.com>
> Date: Fri, 19 Jul 2013 07:29:47 -0700
> 
> The change itself looks good; care to write it up as a proper patch
> with a proposed log message?

My apologies for the delay; I've had to do some yak-shaving to learn
how to construct patches properly.  (I've written some clarifications
for Document/SubmittingPatches, which I will submit separately.)

Someone has gone ahead and made the code change, so all that remains
is to update the testsuite test by replacing the helper program
'test-close-fd-0' with the Posix shell redirection '<&-'.

Dale


 Makefile          |    1 -
 test-close-fd-0.c |   14 --------------
 2 files changed, 0 insertions(+), 15 deletions(-)
 delete mode 100644 test-close-fd-0.c

diff --git a/Makefile b/Makefile
index 8ad40d4..3588ca1 100644
--- a/Makefile
+++ b/Makefile
@@ -557,7 +557,6 @@ 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/test-close-fd-0.c b/test-close-fd-0.c
deleted file mode 100644
index 3745c34..0000000
--- a/test-close-fd-0.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#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;
-}
-- 
1.7.7.6

Re: [PATCH] git_mkstemps: improve test suite test

[Jonathan Nieder]

Hi,

Dale R. Worley wrote:

> Commit 52749 fixes a bug regarding testing the return of an open()

 $ git show 52749
 fatal: ambiguous argument '52749': unknown revision or path not in the working tree.

Could you mention its subject line or date so it's easier to find?

> call for success/failure.  Improve the testsuite test for that fix by
> removing the helper program 'test-close-fd-0' and replacing it with
> the shell redirection '<&-'.  (The redirection is Posix, so it should
> be portable.)
> 
> Signed-off-by: Dale Worley <worley@ariadne.com>
[...]
> Someone has gone ahead and made the code change, so all that remains
> is to update the testsuite test by replacing the helper program
> 'test-close-fd-0' with the Posix shell redirection '<&-'.

The above paragraph should be part of the commit message, since
otherwise the patch is hard to understand.

The patch text looks good.

Thanks and hope that helps,
Jonathan

Re: [PATCH] git_mkstemps: improve test suite test

[Junio C Hamano]

worley@alum.mit.edu (Dale R. Worley) writes:

> Commit 52749 fixes a bug regarding testing the return of an open()
> call for success/failure.  Improve the testsuite test for that fix by
> removing the helper program 'test-close-fd-0' and replacing it with
> the shell redirection '<&-'.  (The redirection is Posix, so it should
> be portable.)
>
> Signed-off-by: Dale Worley <worley@ariadne.com>
> ---

Sorry, but I have no idea what commit you are talking about, and as
far as I can see there is no such file test-close-fd-0.c in my tree.

Puzzled...

>  Makefile          |    1 -
>  test-close-fd-0.c |   14 --------------
>  2 files changed, 0 insertions(+), 15 deletions(-)
>  delete mode 100644 test-close-fd-0.c

Re: [PATCH] git_mkstemps: improve test suite test

[Junio C Hamano]

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

> worley@alum.mit.edu (Dale R. Worley) writes:
>
>> Commit 52749 fixes a bug regarding testing the return of an open()
>> call for success/failure.  Improve the testsuite test for that fix by
>> removing the helper program 'test-close-fd-0' and replacing it with
>> the shell redirection '<&-'.  (The redirection is Posix, so it should
>> be portable.)
>>
>> Signed-off-by: Dale Worley <worley@ariadne.com>
>> ---
>
> Sorry, but I have no idea what commit you are talking about, and as
> far as I can see there is no such file test-close-fd-0.c in my tree.
>
> Puzzled...

OK, let's do this on top of a77f106c (run-command: dup_devnull():
guard against syscalls failing, 2013-07-12) which is at the tip of
tr/fd-gotcha-fixes that contains the earlier fix.

-- >8 --
From: "Dale R. Worley" <worley@alum.mit.edu>
Date: Fri, 2 Aug 2013 20:27:23 -0400
Subject: [PATCH] t0070: test that git_mkstemps correctly checks return value of open()

Signed-off-by: Dale R. Worley <worley@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0070-fundamental.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index da2c504..ff3776f 100755
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -25,6 +25,10 @@ test_expect_success POSIXPERM 'mktemp to unwritable directory prints filename' '
 	grep "cannotwrite/test" err
 '
 
+test_expect_success 'git_mkstemps_mode does not fail if fd 0 is not open' '
+	git commit --allow-empty -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
-- 
1.8.4-rc1-129-g1f3472b