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

[PATCH 3/5] run-command: Elaborate execvp error checking

From
Frans Klaver <fransklaver@gmail.com>
Date
Jan 24, 2012, 22:32 UTC
Message-ID
<1327444346-6243-4-git-send-email-fransklaver@gmail.com>
In-Reply-To
<1327444346-6243-1-git-send-email-fransklaver@gmail.com>

The interpretation of errors from execvp was rather terse. For user convenience communication of the nature of the error can be improved.

This patch introduces a function with more elaborate investigation of the errors encountered. After a failure occurs on a program or script, inspect_failure will run through the PATH entries if necessary, and if the file exists, perform some tests on the file. If nothing is found, ENOENT is returned.

The function inspect_file() will try to find out if the file itself or an interpreter caused the issue. Some scripts don't have the hash-bang and will not be reported as scripts. If the interpreter cannot be found, the existing behavior is kept -- i.e. ENOENT will be reported to the parent process.

Signed-off-by: Frans Klaver <fransklaver@gmail.com>
---
 run-command.c          |  144 +++++++++++++++++++++++++++++++++++++++++++++---
 t/t0061-run-command.sh |    6 +-
 2 files changed, 140 insertions(+), 10 deletions(-)
diff --git a/run-command.c b/run-command.c
index 1c51043..17a65fe 100644
--- a/run-command.c
+++ b/run-command.c
@@ -2,6 +2,7 @@
 #include "run-command.h"
 #include "exec_cmd.h"
 #include "argv-array.h"
+#include "dir.h"
 
 static inline void close_pair(int fd[2])
 {
@@ -134,6 +135,140 @@ static int wait_or_whine(pid_t pid, const char *argv0, int silent_exec_failure)
 	return code;
 }
 
+#ifndef WIN32
+static int is_executable_file(const char *path)
+{
+	return access(path, X_OK) == 0 && !is_directory(path);
+}
+
+static int is_searchable(const char *path)
+{
+	return access(path, X_OK) == 0 && is_directory(path);
+}
+
+static void die_file_error(const char *file, int err)
+{
+	die("cannot exec '%s': %s", file, strerror(err));
+}
+
+static char *get_interpreter(const char *first_line)
+{
+	struct strbuf sb = STRBUF_INIT;
+	size_t start = strspn(first_line + 2, " \t") + 2;
+	size_t end = strcspn(first_line + start, " \t\r\n") + start;
+
+	if (start >= end)
+		return NULL;
+
+	strbuf_add(&sb, first_line + start, end - start);
+	return strbuf_detach(&sb, NULL);
+}
+
+static void inspect_file(struct strbuf *fn, int err, const char *argv0)
+{
+	/*
+	 * Typical line length of 80. BSD only allows 32 bytes, but that
+	 * won't really make a difference.
+	 */
+	char buf[80];
+	size_t read;
+	FILE *file;
+	if (!is_executable_file(fn->buf)) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	file = fopen(fn->buf, "r");
+	if (!file) {
+		strbuf_release(fn);
+		die_file_error(argv0, err);
+	}
+
+	read = fread(buf, 1, sizeof(buf), file);
+	fclose(file);
+	if (read > 2 && buf[0] == '#' && buf[1] == '!') {
+		char *i = get_interpreter(buf);
+		char es[1024];
+		if (i) {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter '%s': %s",
+					argv0, i,
+					strerror(err));
+		} else {
+			sprintf(es, "cannot exec '%s': "
+					"bad interpreter: %s",
+					argv0, strerror(err));
+		}
+		free(i);
+		strbuf_release(fn);
+		if (err == ENOENT) {
+			error("%s", es);
+			exit(127);
+		} else {
+			die("%s", es);
+		}
+	}
+
+	strbuf_release(fn);
+	die_file_error(argv0, err);
+}
+
+static void inspect_failure(const char *argv0, int silent_exec_failure)
+{
+	int err = errno;
+	struct strbuf sb = STRBUF_INIT;
+
+	/* errors not related to path */
+	if (errno == E2BIG || errno == ENOMEM)
+		die_file_error(argv0, err);
+
+	if (strchr(argv0, '/')) {
+		if (file_exists(argv0)) {
+			strbuf_add(&sb, argv0, strlen(argv0));
+			inspect_file(&sb, err, argv0);
+		}
+	} else {
+		char *path, *next;
+		path = getenv("PATH");
+		while (path) {
+			next = strchrnul(path, ':');
+			if (path < next)
+				strbuf_add(&sb, path, next - path);
+			else
+				strbuf_addch(&sb, '.');
+
+			if (!*next)
+				path = NULL;
+			else
+				path = next + 1;
+
+			if (!is_searchable(sb.buf)) {
+				strbuf_release(&sb);
+				continue;
+			}
+
+			if (sb.len && sb.buf[sb.len - 1] != '/')
+				strbuf_addch(&sb, '/');
+			strbuf_addstr(&sb, argv0);
+
+			if (file_exists(sb.buf))
+				inspect_file(&sb, err, argv0);
+
+			strbuf_release(&sb);
+		}
+	}
+
+	if (err == ENOENT) {
+		if (!silent_exec_failure)
+			error("cannot exec '%s': %s", argv0,
+					strerror(ENOENT));
+		exit(127);
+	} else {
+		die_file_error(argv0, err);
+	}
+}
+#endif
+
 int start_command(struct child_process *cmd)
 {
 	int need_in, need_out, need_err;
@@ -280,14 +415,7 @@ fail_pipe:
 		} else {
 			execvp(cmd->argv[0], (char *const*) cmd->argv);
 		}
-		if (errno == ENOENT) {
-			if (!cmd->silent_exec_failure)
-				error("cannot run %s: %s", cmd->argv[0],
-					strerror(ENOENT));
-			exit(127);
-		} else {
-			die_errno("cannot exec '%s'", cmd->argv[0]);
-		}
+		inspect_failure(cmd->argv[0], cmd->silent_exec_failure);
 	}
 	if (cmd->pid < 0)
 		error("cannot fork() for %s: %s", cmd->argv[0],
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
index 31eb3c3..14b4ea6 100755
--- a/t/t0061-run-command.sh
+++ b/t/t0061-run-command.sh
@@ -71,7 +71,8 @@ test_expect_success POSIXPERM 'run_command reports EACCES, interpreter fails' '
 	chmod -x someinterpreter &&
 	test_must_fail test-run-command run-command ./hello.sh 2>err &&
 
-	grep "fatal: cannot exec.*hello.sh" err
+	grep "fatal: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
@@ -79,7 +80,8 @@ test_expect_failure POSIXPERM 'run_command reports ENOENT, interpreter' '
 	chmod +x hello.sh &&
 	test_must_fail test-run-command start-command-ENOENT ./hello.sh 2>err &&
 
-	grep "error: cannot exec.*hello.sh" err
+	grep "error: cannot exec.*hello.sh" err &&
+	grep "bad interpreter" err
 '
 
 test_done
-- 
1.7.8.1
Previous: Frans KlaverNext: Jonathan Nieder
Message 12 of 31 in “Add execvp failure diagnostics”
  1. 0/6 Add execvp failure diagnosticsFrans Klaver, Jan 24, 2012
  2. 1/5 t0061: Fix incorrect indentationFrans Klaver, Jan 24, 2012
  3. Junio C HamanoJan 24, 2012
  4. Jonathan NiederJan 24, 2012
  5. Frans KlaverJan 25, 2012
  6. Junio C HamanoJan 25, 2012
  7. Frans KlaverJan 25, 2012
  8. Frans KlaverJan 25, 2012
  9. 2/5 t0061: Add testsFrans Klaver, Jan 24, 2012
  10. Jonathan NiederJan 24, 2012
  11. Frans KlaverJan 25, 2012
  12. 3/5 run-command: Elaborate execvp error checkingFrans Klaver, Jan 24, 2012
  13. Jonathan NiederJan 24, 2012
  14. Frans KlaverJan 25, 2012
  15. Jonathan NiederJan 25, 2012
  16. Frans KlaverJan 25, 2012
  17. Johannes SixtJan 25, 2012
  18. Frans KlaverJan 25, 2012
  19. 4/5 run-command: Warn if PATH entry cannot be searchedFrans Klaver, Jan 24, 2012
  20. 5/5 run-command: Error out if interpreter not foundFrans Klaver, Jan 24, 2012
  21. Jonathan NiederJan 24, 2012
  22. Frans KlaverJan 25, 2012
  23. Johannes SixtJan 25, 2012
  24. Frans KlaverJan 25, 2012
  25. Junio C HamanoJan 26, 2012
  26. Frans KlaverJan 27, 2012
  27. Jonathan NiederJan 27, 2012
  28. Frans KlaverJan 27, 2012
  29. Jonathan NiederJan 27, 2012
  30. Frans KlaverJan 27, 2012
  31. Frans KlaverFeb 4, 2012

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.