run_command: return exit code as positive value

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-merge.c b/builtin-merge.c
index af9adab300de98026b23a58b1d8b40f9395ff11a..96ecaf4e484639c844d48884ae2fd559d5e9a352 100644 (file)
--- 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 6ec1d056e6fa24bf6008d9114df5d5cdacd837af..6235903552ea6ea9743717cbd7a172050862bc68 100644 (file)
--- 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 1816e977b7b13782003fc78a9de9b47cd3554a32..491e7141b4ea29b3cf754cbaf2656a0c3ca8c46c 100644 (file)
--- 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 65ed733fda86364b88a961093d5021c6d09934a6..d223eab62f0b4e6a9d2e17edfbad4245c17b63d4 100644 (file)
--- 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 a2c13c4c087f7b4961f0507783d34d19ff4b2921..31c74578f6b82d859e038ddf9ca43d3b5fd9429a 100644 (file)
--- 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 eb2efc33073512efd14b65e67db8cf814ca3fa33..a4e309eeb9b1e92dea7a4c179276a0a45f8064c7 100644 (file)
--- 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 e3455028435eab958d5f86a3e86249f1704b9c1b..0211e1d471d37a41f77098e8fff8a031fb26cb71 100644 (file)
--- 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;