From: Johannes Sixt <j6t@kdbg.org>
Date: Tue, 27 Jan 2009 10:06:40 GMT
Subject: Re: [RFC/PATCH 0/3] fix "Funny: git -p submodule summary"
Message-ID: <497EDCB0.8080806@kdbg.org>
In-Reply-To: <20090127062512.GA10487@coredump.intra.peff.net>

Jeff King schrieb:
> JSixt, there are some tweaks to the Windows code to report back the exec
> error. They look obviously correct to me, but I have no box to test
> (even compile test) them on.

Generally, I like this series, in particular since it does not degrade 
performance on Windows ;)

But the following is needed in addition.

- Note that run_command returns the negated exit code, therefore, we 
must negate it again in the call to exit(). Without this t6030 failed. 
(And negative exit codes causes grief on Windows because bash for some 
reason does not recognize that as failure.)

- The close() calls can overwrite errno. And since fork() should not 
(cannot?) fail with ENOENT, it's safe to remove the #ifdef __MINGW32__.

-- Hannes

diff --git a/git.c b/git.c
index 79a836c..35635d1 100644
--- a/git.c
+++ b/git.c
@@ -414,7 +414,7 @@ static void execv_dashed_external
  	 */
  	status = run_command_v_opt(argv, 0);
  	if (status != -ERR_RUN_COMMAND_EXEC)
-		exit(status);
+		exit(-status);
  	errno = ENOENT; /* as if we called execvp */

  	argv[0] = tmp;
diff --git a/run-command.c b/run-command.c
index 2437798..b05c734 100644
--- a/run-command.c
+++ b/run-command.c
@@ -187,27 +187,24 @@ int start_command
  	if (s2 >= 0)
  		dup2(s2, 2), close(s2);
  #endif

  	if (cmd->pid < 0) {
+		int err = errno;
  		if (need_in)
  			close_pair(fdin);
  		else if (cmd->in)
  			close(cmd->in);
  		if (need_out)
  			close_pair(fdout);
  		else if (cmd->out)
  			close(cmd->out);
  		if (need_err)
  			close_pair(fderr);
-#ifndef __MINGW32__
-		return -ERR_RUN_COMMAND_FORK;
-#else
-		return errno == ENOENT ?
+		return err == ENOENT ?
  			-ERR_RUN_COMMAND_EXEC :
  			-ERR_RUN_COMMAND_FORK;
-#endif
  	}

  	if (need_in)
  		close(fdin[0]);
  	else if (cmd->in)