git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: Jeff King <peff@peff.net>
Cc: "Clemens Buchacher" <drizzd@aon.at>,
	"Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>,
	git@vger.kernel.org, gitster@pobox.com
Subject: Re: [PATCH] t5570: forward git-daemon messages in a different way
Date: Thu, 26 Apr 2012 20:16:37 +0200	[thread overview]
Message-ID: <4F999105.200@kdbg.org> (raw)
In-Reply-To: <20120426130129.GA27785@sigill.intra.peff.net>

Am 26.04.2012 15:01, schrieb Jeff King:
> On Thu, Apr 19, 2012 at 08:58:01AM +0200, Johannes Sixt wrote:
> 
>>> What? The shell is literally redirecting the cat process's stdin from
>>> /dev/null. I'm totally confused.
>>
>> You don't have to be; it's mandated by POSIX:
>>
>> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_03_02
> 
> Sorry for the delayed response.
> 
> Thanks for the pointer; I looked in POSIX but couldn't find that
> passage. It does say "In all cases, explicit redirection of standard
> input shall override this activity". It looks like dash interprets that
> as "open /dev/null, then open the redirected stdin". Which leaves a race
> condition.

I don't see a race condition. The specs are clear: First redirect stdin
to /dev/null, and if there are other redirections, apply them later.
But in our code we have only:

	cat >&4 &

i.e., there are no other redirections. It does not matter that the
whole block where this command occurs is redirected from
git_daemon_output; that redirection was applied before the command was
executed, and it was already overridden by the implicit /dev/null
redirection.

>  So I think a custom wrapper like the one posted by Clemens
> is our only portable option.

I don't think so. How about this?

diff --git a/t/lib-git-daemon.sh b/t/lib-git-daemon.sh
index ef2d01f..7245ab3 100644
--- a/t/lib-git-daemon.sh
+++ b/t/lib-git-daemon.sh
@@ -30,10 +30,10 @@ start_git_daemon() {
 		"$@" "$GIT_DAEMON_DOCUMENT_ROOT_PATH" \
 		>&3 2>git_daemon_output &
 	GIT_DAEMON_PID=$!
+	exec 7<git_daemon_output &&
 	{
-		read line
+		read line <&7
 		echo >&4 "$line"
-		cat >&4 &
 
 		# Check expected output
 		if test x"$(expr "$line" : "\[[0-9]*\] \(.*\)")" != x"Ready to rumble"
@@ -43,7 +43,9 @@ start_git_daemon() {
 			trap 'die' EXIT
 			error "git daemon failed to start"
 		fi
-	} <git_daemon_output
+		cat <&7 >&4 &
+		exec 7<&-
+	}
 }
 
 stop_git_daemon() {


i.e., we open the readable end of the pipe in the shell, and dup
it from there to 'read' and later to 'cat'. Finally, we can
close it, because 'cat' has it still open in the background.

This works with dash, bash, and (half-way through) ksh. (The failure
with ksh is an unrelated problem.)

-- Hannes

  reply	other threads:[~2012-04-26 18:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-14  8:44 [PATCH] t5570: forward git-daemon messages in a different way Zbigniew Jędrzejewski-Szmek
2012-04-14 12:13 ` Clemens Buchacher
2012-04-14 12:21   ` Clemens Buchacher
2012-04-16 15:43     ` Zbigniew Jędrzejewski-Szmek
2012-04-16 17:09       ` Junio C Hamano
2012-04-16 21:22         ` Clemens Buchacher
2012-04-16 22:06           ` Zbigniew Jędrzejewski-Szmek
2012-04-17 15:43             ` Junio C Hamano
2012-04-16 17:42       ` Jeff King
2012-04-16 22:44         ` Clemens Buchacher
2012-04-19  6:03           ` Jeff King
2012-04-19  6:58             ` Johannes Sixt
2012-04-26 13:01               ` Jeff King
2012-04-26 18:16                 ` Johannes Sixt [this message]
2012-04-26 19:55                   ` Clemens Buchacher
2012-04-26 21:00                     ` [PATCH] t5570: fix forwarding of git-daemon messages via cat Johannes Sixt
2012-04-26 21:10                       ` Zbigniew Jędrzejewski-Szmek
2012-04-27  7:59                       ` Jeff King
2012-04-27 15:02                       ` Junio C Hamano
2012-04-27  7:55                   ` [PATCH] t5570: forward git-daemon messages in a different way Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F999105.200@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=drizzd@aon.at \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=zbyszek@in.waw.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).