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

[PATCH 3/7] run_command: return exit code as positive value

From
Johannes Sixt <j6t@kdbg.org>
Date
Jul 4, 2009, 19:26 UTC
Message-ID
<4fe5ad61e7500735d1bbc12c98a863dd3499ea31.1246734159.git.j6t@kdbg.org>
In-Reply-To
<d63e9230d57698a058c8a550709155e5e3222348.1246734159.git.j6t@kdbg.org>

As a general guideline, functions in git's code return zero to indicate success and negative values to indicate failure. The run_command family of functions followed this guideline. But there are actually two different kinds of failure:

- failures of system calls;
- non-zero exit code of the program that was run.

Usually, a non-zero exit code of the program is a failure and means a failure to the caller. Except that sometimes it does not. For example, the exit code of merge programs (e.g. external merge drivers) conveys information about how the merge failed, and not all exit calls are actually failures.

Furthermore, the return value of run_command is sometimes used as exit code by the caller.

This change arranges that the exit code of the program is returned as a positive value, which can now be regarded as the "result" of the function. System call failures continue to be reported as negative values.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin-merge.c        |    2 +-
 builtin-receive-pack.c |    4 ++--
 convert.c              |    2 +-
 git.c                  |    4 ++--
 ll-merge.c             |    4 ----
 run-command.c          |    9 +--------
 run-command.h          |    1 -
 7 files changed, 7 insertions(+), 19 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index af9adab..96ecaf4 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -594,7 +594,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 		discard_cache();
 		if (read_cache() < 0)
 			die("failed to read the cache");
-		return -ret;
+		return ret;
 	}
 }
 
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 6ec1d05..6235903 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -143,8 +143,8 @@ static int run_status(int code, const char *cmd_name)
 	case -ERR_RUN_COMMAND_WAITPID_NOEXIT:
 		return error("%s died strangely", cmd_name);
 	default:
-		error("%s exited with error code %d", cmd_name, -code);
-		return -code;
+		error("%s exited with error code %d", cmd_name, code);
+		return code;
 	}
 }
 
diff --git a/convert.c b/convert.c
index 1816e97..491e714 100644
--- a/convert.c
+++ b/convert.c
@@ -267,7 +267,7 @@ static int filter_buffer(int fd, void *data)
 
 	status = finish_command(&child_process);
 	if (status)
-		error("external filter %s failed %d", params->cmd, -status);
+		error("external filter %s failed %d", params->cmd, status);
 	return (write_err || status);
 }
 
diff --git a/git.c b/git.c
index f4d53f4..662f21e 100644
--- a/git.c
+++ b/git.c
@@ -418,9 +418,9 @@ static void execv_dashed_external(const char **argv)
 	 */
 	status = run_command_v_opt(argv, 0);
 	if (status != -ERR_RUN_COMMAND_EXEC) {
-		if (IS_RUN_COMMAND_ERR(status))
+		if (status < 0)
 			die("unable to run '%s'", argv[0]);
-		exit(-status);
+		exit(status);
 	}
 	errno = ENOENT; /* as if we called execvp */
 
diff --git a/ll-merge.c b/ll-merge.c
index a2c13c4..31c7457 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -192,10 +192,6 @@ static int ll_ext_merge(const struct ll_merge_driver *fn,
 
 	args[2] = cmd.buf;
 	status = run_command_v_opt(args, 0);
-	if (status < -ERR_RUN_COMMAND_FORK)
-		; /* failure in run-command */
-	else
-		status = -status;
 	fd = open(temp[1], O_RDONLY);
 	if (fd < 0)
 		goto bad;
diff --git a/run-command.c b/run-command.c
index eb2efc3..a4e309e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -241,14 +241,7 @@ static int wait_or_whine(pid_t pid)
 		if (!WIFEXITED(status))
 			return -ERR_RUN_COMMAND_WAITPID_NOEXIT;
 		code = WEXITSTATUS(status);
-		switch (code) {
-		case 127:
-			return -ERR_RUN_COMMAND_EXEC;
-		case 0:
-			return 0;
-		default:
-			return -code;
-		}
+		return code == 127 ? -ERR_RUN_COMMAND_EXEC : code;
 	}
 }
 
diff --git a/run-command.h b/run-command.h
index e345502..0211e1d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -10,7 +10,6 @@ enum {
 	ERR_RUN_COMMAND_WAITPID_SIGNAL,
 	ERR_RUN_COMMAND_WAITPID_NOEXIT,
 };
-#define IS_RUN_COMMAND_ERR(x) (-(x) >= ERR_RUN_COMMAND_FORK)
 
 struct child_process {
 	const char **argv;
-- 
1.6.3.17.g1665f
Previous: Johannes SixtNext: Johannes Sixt
Message 4 of 12 in “rework run_command error reporting”
  1. 0/7 rework run_command error reportingJohannes Sixt, Jul 4, 2009
  2. 1/7 Truncate result of run_command that is used in exit() to lowest 8 bitsJohannes Sixt, Jul 4, 2009
  3. 2/7 MinGW: simplify waitpid() emulation macrosJohannes Sixt, Jul 4, 2009
  4. 3/7 run_command: return exit code as positive valueJohannes Sixt, Jul 4, 2009
  5. 4/7 run_command: report system call errors instead of returning error codesJohannes Sixt, Jul 4, 2009
  6. 5/7 run_command: encode deadly signal number in the return valueJohannes Sixt, Jul 4, 2009
  7. 6/7 run_command: report failure to execute the program, but optionally don'tJohannes Sixt, Jul 4, 2009
  8. 7/7 receive-pack: remove unnecessary run_status reportJohannes Sixt, Jul 4, 2009
  9. Johannes SixtJul 5, 2009
  10. Junio C HamanoJul 5, 2009
  11. Johannes SixtJul 6, 2009
  12. 1/7 MinGW: truncate exit()'s argument to lowest 8 bitsJohannes Sixt, Jul 5, 2009

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.