diff: drop useless "status" parameter from diff_result_code()

Many programs use diff_result_code() to get a user-visible program exit code from a diff result (e.g., checking opts.found_changes if --exit-code was requested). This function also takes a "status" parameter, which seems at first glance that it could be used to propagate an error encountered when computing the diff. But it doesn't work that way: - negative values are passed through as-is, but are not appropriate as program exit codes - when --exit-code or --check is in effect, we _ignore_ the passed-in status completely. So a failed diff which did not have a chance to set opts.found_changes would erroneously report "success, no changes" instead of propagating the error. After recent cleanups, neither of these bugs is possible to trigger, as every caller just passes in "0". So rather than fixing them, we can simply drop the useless parameter instead. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 21, 2023 at 16:20 UTC 5cc6b2d70bc55ab75913ee93d9ac96ad875fbb29
12 files changed +16 -18
builtin/describe.c
+1 -1
@@ -687,7 +687,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
687 BUG("malformed internal diff-index command line");
688 run_diff_index(&revs, 0);
689
690 - if (!diff_result_code(&revs.diffopt, 0))
690 + if (!diff_result_code(&revs.diffopt))
691 suffix = NULL;
692 else
693 suffix = dirty;
builtin/diff-files.c
+1 -1
@@ -83,7 +83,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
83 if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0)
84 die_errno("repo_read_index_preload");
85 run_diff_files(&rev, options);
86 - result = diff_result_code(&rev.diffopt, 0);
86 + result = diff_result_code(&rev.diffopt);
87 release_revisions(&rev);
88 return result;
89 }
builtin/diff-index.c
+1 -1
@@ -73,7 +73,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
73 return -1;
74 }
75 run_diff_index(&rev, option);
76 - result = diff_result_code(&rev.diffopt, 0);
76 + result = diff_result_code(&rev.diffopt);
77 release_revisions(&rev);
78 return result;
79 }
builtin/diff-tree.c
+1 -1
@@ -232,5 +232,5 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
232 diff_free(&opt->diffopt);
233 }
234
235 - return diff_result_code(&opt->diffopt, 0);
235 + return diff_result_code(&opt->diffopt);
236 }
builtin/diff.c
+1 -1
@@ -608,7 +608,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
608 builtin_diff_combined(&rev, argc, argv,
609 ent.objects, ent.nr,
610 first_non_parent);
611 - result = diff_result_code(&rev.diffopt, 0);
611 + result = diff_result_code(&rev.diffopt);
612 if (1 < rev.diffopt.skip_stat_unmatch)
613 refresh_index_quietly();
614 release_revisions(&rev);
builtin/log.c
+1 -1
@@ -549,7 +549,7 @@ static int cmd_log_walk_no_free(struct rev_info *rev)
549 rev->diffopt.flags.check_failed) {
550 return 02;
551 }
552 - return diff_result_code(&rev->diffopt, 0);
552 + return diff_result_code(&rev->diffopt);
553 }
554
555 static int cmd_log_walk(struct rev_info *rev)
builtin/stash.c
+3 -3
@@ -973,7 +973,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)
973 }
974 log_tree_diff_flush(&rev);
975
976 - ret = diff_result_code(&rev.diffopt, 0);
976 + ret = diff_result_code(&rev.diffopt);
977 cleanup:
978 strvec_clear(&stash_args);
979 free_stash_info(&info);
@@ -1111,13 +1111,13 @@ static int check_changes_tracked_files(const struct pathspec *ps)
1111 diff_setup_done(&rev.diffopt);
1112
1113 run_diff_index(&rev, DIFF_INDEX_CACHED);
1114 - if (diff_result_code(&rev.diffopt, 0)) {
1114 + if (diff_result_code(&rev.diffopt)) {
1115 ret = 1;
1116 goto done;
1117 }
1118
1119 run_diff_files(&rev, 0);
1120 - if (diff_result_code(&rev.diffopt, 0)) {
1120 + if (diff_result_code(&rev.diffopt)) {
1121 ret = 1;
1122 goto done;
1123 }
builtin/submodule--helper.c
+1 -1
@@ -670,7 +670,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
670 setup_revisions(diff_files_args.nr, diff_files_args.v, &rev, &opt);
671 run_diff_files(&rev, 0);
672
673 - if (!diff_result_code(&rev.diffopt, 0)) {
673 + if (!diff_result_code(&rev.diffopt)) {
674 print_status(flags, ' ', path, ce_oid,
675 displaypath);
676 } else if (!(flags & OPT_CACHED)) {
diff-no-index.c
+1 -1
@@ -364,7 +364,7 @@ int diff_no_index(struct rev_info *revs,
364 * The return code for --no-index imitates diff(1):
365 * 0 = no changes, 1 = changes, else error
366 */
367 - ret = diff_result_code(&revs->diffopt, 0);
367 + ret = diff_result_code(&revs->diffopt);
368
369 out:
370 for (i = 0; i < ARRAY_SIZE(to_free); i++)
diff.c
+2 -4
@@ -6973,16 +6973,14 @@ void diffcore_std(struct diff_options *options)
6973 options->found_follow = 0;
6974 }
6975
6976 -int diff_result_code(struct diff_options *opt, int status)
6976 +int diff_result_code(struct diff_options *opt)
6977 {
6978 int result = 0;
6979
6980 diff_warn_rename_limit("diff.renameLimit",
6981 opt->needed_rename_limit,
6982 opt->degraded_cc_to_c);
6983 - if (!opt->flags.exit_with_status &&
6984 - !(opt->output_format & DIFF_FORMAT_CHECKDIFF))
6985 - return status;
6983 +
6984 if (opt->flags.exit_with_status &&
6985 opt->flags.has_changes)
6986 result |= 01;
diff.h
+1 -1
@@ -647,7 +647,7 @@ int do_diff_cache(const struct object_id *, struct diff_options *);
647 int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
648 void flush_one_hunk(struct object_id *result, git_hash_ctx *ctx);
649
650 -int diff_result_code(struct diff_options *, int);
650 +int diff_result_code(struct diff_options *);
651
652 int diff_no_index(struct rev_info *,
653 int implicit_no_index, int, const char **);
wt-status.c
+2 -2
@@ -2581,7 +2581,7 @@ int has_unstaged_changes(struct repository *r, int ignore_submodules)
2581 rev_info.diffopt.flags.quick = 1;
2582 diff_setup_done(&rev_info.diffopt);
2583 run_diff_files(&rev_info, 0);
2584 - result = diff_result_code(&rev_info.diffopt, 0);
2584 + result = diff_result_code(&rev_info.diffopt);
2585 release_revisions(&rev_info);
2586 return result;
2587 }
@@ -2615,7 +2615,7 @@ int has_uncommitted_changes(struct repository *r,
2615
2616 diff_setup_done(&rev_info.diffopt);
2617 run_diff_index(&rev_info, DIFF_INDEX_CACHED);
2618 - result = diff_result_code(&rev_info.diffopt, 0);
2618 + result = diff_result_code(&rev_info.diffopt);
2619 release_revisions(&rev_info);
2620 return result;
2621 }