git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[Updated PATCH 1/2] Report exec errors from run-command

From
ILIlari Liusvaara <ilari.liusvaara@elisanet.fi>
Date
Dec 30, 2009, 10:52 UTC
Message-ID
<1262170338-11574-2-git-send-email-ilari.liusvaara@elisanet.fi>
In-Reply-To
<1262170338-11574-1-git-send-email-ilari.liusvaara@elisanet.fi>

Previously run-command was unable to report errors happening in exec call. Change it to pass errno from failed exec to errno value at return.

The errno value passing can be done by opening close-on-exec pipe and piping the error code through in case of failure. In case of success, close-on-exec closes the pipe on successful exec and parent process gets end of file on read.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
 Makefile               |    1 +
 run-command.c          |   79 ++++++++++++++++++++++++++++++++++++++++++++++--
 t/t0061-run-command.sh |   13 ++++++++
 test-run-command.c     |   35 +++++++++++++++++++++
 4 files changed, 125 insertions(+), 3 deletions(-)
 create mode 100755 t/t0061-run-command.sh
 create mode 100644 test-run-command.c
diff --git a/Makefile b/Makefile
index 4a1e5bc..452ad50 100644
--- a/Makefile
+++ b/Makefile
@@ -1725,6 +1725,7 @@ TEST_PROGRAMS += test-parse-options$X
 TEST_PROGRAMS += test-path-utils$X
 TEST_PROGRAMS += test-sha1$X
 TEST_PROGRAMS += test-sigchain$X
+TEST_PROGRAMS += test-run-command$X
 
 all:: $(TEST_PROGRAMS)
 
diff --git a/run-command.c b/run-command.c
index cf2d8f7..34e5af4 100644
--- a/run-command.c
+++ b/run-command.c
@@ -15,6 +15,12 @@ static inline void dup_devnull(int to)
 	close(fd);
 }
 
+static inline void force_close(int fd)
+{
+	while (close(fd) < 0 && errno != EBADF)
+		; /* No-op */
+}
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
@@ -76,9 +82,64 @@ fail_pipe:
 	trace_argv_printf(cmd->argv, "trace: run_command:");
 
 #ifndef WIN32
+{
+	int report_pipe[2] = {-1, -1};
+
+	if (pipe(report_pipe) < 0) {
+		report_pipe[0] = -1;
+		report_pipe[1] = -1;
+		warning("Can't open pipe for exec status report: %s\n",
+			strerror(errno));
+	}
+
 	fflush(NULL);
 	cmd->pid = fork();
-	if (!cmd->pid) {
+	if (cmd->pid > 0) {
+		int r = 0, ret;
+		force_close(report_pipe[1]);
+read_again:
+		if (report_pipe[0] > 0)
+			r = read(report_pipe[0], &ret, sizeof(ret));
+		if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+			errno == EWOULDBLOCK))
+			goto read_again;
+		else if (r < 0)
+			warning("Can't read exec status report: %s\n",
+				strerror(errno));
+		else if (r == 0)
+			;
+		else if (r < sizeof(ret)) {
+			warning("Received incomplete exec status report.\n");
+			errno = EBADMSG;
+		} else {
+			failed_errno = ret;
+			/*
+			 * Clean up the process that did the failed execution
+			 * so no zombies remain.
+			 */
+wait_again:
+			r = waitpid(cmd->pid, &ret, 0);
+			if (r < 0 && errno != ECHILD)
+				goto wait_again;
+			cmd->pid = -1;
+		}
+	} else if (!cmd->pid) {
+		int r = 0;
+		int flags;
+		force_close(report_pipe[0]);
+
+		flags = fcntl(report_pipe[1], F_GETFD);
+		if (flags < 0)
+			r = -1;
+		flags |= FD_CLOEXEC;
+		r = (r || fcntl(report_pipe[1], F_SETFD, flags));
+		if (r) {
+			warning("Can't FD_CLOEXEC pipe for exec status "
+				"report: %s\n", strerror(errno));
+			force_close(report_pipe[1]);
+			report_pipe[1] = -1;
+		}
+
 		if (cmd->no_stdin)
 			dup_devnull(0);
 		else if (need_in) {
@@ -126,13 +187,25 @@ fail_pipe:
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
+		failed_errno = errno;
+
 		trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
 				strerror(errno));
+
+		r = 0;
+write_again:
+		if (report_pipe[1] >= 0)
+			r = write(report_pipe[1], &failed_errno,
+				sizeof(failed_errno));
+		if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+			errno == EWOULDBLOCK))
+			goto write_again;
+
 		exit(127);
-	}
-	if (cmd->pid < 0)
+	} else if (cmd->pid < 0)
 		error("cannot fork() for %s: %s", cmd->argv[0],
 			strerror(failed_errno = errno));
+}
 #else
 {
 	int s0 = -1, s1 = -1, s2 = -1;	/* backups of stdin, stdout, stderr */
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
new file mode 100755
index 0000000..1d9e82a
--- /dev/null
+++ b/t/t0061-run-command.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Ilari Liusvaara
+#
+
+test_description='Test run command'
+
+. ./test-lib.sh
+
+test_expect_success "reporting ENOENT" \
+"test-run-command 1"
+
+test_done
diff --git a/test-run-command.c b/test-run-command.c
new file mode 100644
index 0000000..6297785
--- /dev/null
+++ b/test-run-command.c
@@ -0,0 +1,35 @@
+/*
+ * test-run-command.c: test run command API.
+ *
+ * (C) 2009 Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
+ *
+ * This code is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "git-compat-util.h"
+#include "run-command.h"
+#include <string.h>
+#include <errno.h>
+
+int main(int argc, char **argv)
+{
+	char* procs[2];
+	struct child_process proc;
+	memset(&proc, 0, sizeof(proc));
+
+	if(argc < 2)
+		return 1;
+
+	if (argv[1][1] == '1')
+		procs[0] = "does-not-exist-62896869286";
+	procs[1] = NULL;
+	proc.argv = (const char **)procs;
+
+	if (!run_command(&proc))
+		return 1;
+	if (errno != ENOENT)
+		return 1;
+	return 0;
+}
-- 
1.6.6.3.gaa2e1
Previous: Ilari LiusvaaraNext: Erik Faye-Lund
Message 2 of 13 in “Improve remote helpers exec error reporting”
  1. 0/2 Improve remote helpers exec error reportingIlari Liusvaara, Dec 30, 2009
  2. 1/2 Report exec errors from run-commandIlari Liusvaara, Dec 30, 2009
  3. Erik Faye-LundDec 30, 2009
  4. TarmiganDec 31, 2009
  5. Ilari LiusvaaraDec 31, 2009
  6. TarmiganDec 31, 2009
  7. 2/2 Improve transport helper exec failure reportingIlari Liusvaara, Dec 30, 2009
  8. Johannes SixtDec 31, 2009
  9. Ilari LiusvaaraDec 31, 2009
  10. Johannes SixtDec 31, 2009
  11. Ilari LiusvaaraDec 31, 2009
  12. Johannes SixtDec 31, 2009
  13. Johannes SixtJan 1, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.