Skip to content

Commit

Permalink
run_command: return exit code as positive value
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Johannes Sixt authored and Junio C Hamano committed Jul 5, 2009
1 parent 303e7c4 commit 5709e03
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 19 deletions.
2 changes: 1 addition & 1 deletion builtin-merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
4 changes: 2 additions & 2 deletions builtin-receive-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
2 changes: 1 addition & 1 deletion convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 2 additions & 2 deletions git.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down
4 changes: 0 additions & 4 deletions ll-merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 1 addition & 8 deletions run-command.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
1 change: 0 additions & 1 deletion run-command.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 5709e03

Please sign in to comment.