git-commit-vandalism/builtin/diff-files.c
Jeff King 04393ae7f7 diff-files: move misplaced cleanup label
Commit 0139c58ab9 (revisions API users: add "goto cleanup" for
release_revisions(), 2022-04-13) converted an early return in
cmd_diff_files() into a goto. But it put the cleanup label too early: if
read_cache_preload() returns an error, we'll set result to "-1", but
then jump to calling run_diff_files(), overwriting our result.

We should jump past the call to run_diff_files(). Likewise, we should go
past diff_result_code(), which is expecting to see a code from an actual
diff, not a negative error code.

In practice, I suspect this bug cannot actually be triggered, because
read_cache_preload() does not seem to ever return an error. Its return
value (eventually) comes from do_read_index(), which gives the number of
cache entries found, and calls die() on error. Still, it makes sense to
fix the inadvertent change from 0139c58ab9 first, and we can look into
the overall error handling of read_cache() separately (which is present
in many other callsites).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-12 07:17:28 -07:00

89 lines
2.4 KiB
C

/*
* GIT - The information manager from hell
*
* Copyright (C) Linus Torvalds, 2005
*/
#define USE_THE_INDEX_COMPATIBILITY_MACROS
#include "cache.h"
#include "config.h"
#include "diff.h"
#include "diff-merges.h"
#include "commit.h"
#include "revision.h"
#include "builtin.h"
#include "submodule.h"
static const char diff_files_usage[] =
"git diff-files [-q] [-0 | -1 | -2 | -3 | -c | --cc] [<common-diff-options>] [<path>...]"
COMMON_DIFF_OPTIONS_HELP;
int cmd_diff_files(int argc, const char **argv, const char *prefix)
{
struct rev_info rev;
int result;
unsigned options = 0;
if (argc == 2 && !strcmp(argv[1], "-h"))
usage(diff_files_usage);
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
repo_init_revisions(the_repository, &rev, prefix);
rev.abbrev = 0;
/*
* Consider "intent-to-add" files as new by default, unless
* explicitly specified in the command line or anywhere else.
*/
rev.diffopt.ita_invisible_in_index = 1;
prefix = precompose_argv_prefix(argc, argv, prefix);
argc = setup_revisions(argc, argv, &rev, NULL);
while (1 < argc && argv[1][0] == '-') {
if (!strcmp(argv[1], "--base"))
rev.max_count = 1;
else if (!strcmp(argv[1], "--ours"))
rev.max_count = 2;
else if (!strcmp(argv[1], "--theirs"))
rev.max_count = 3;
else if (!strcmp(argv[1], "-q"))
options |= DIFF_SILENT_ON_REMOVED;
else
usage(diff_files_usage);
argv++; argc--;
}
if (!rev.diffopt.output_format)
rev.diffopt.output_format = DIFF_FORMAT_RAW;
rev.diffopt.rotate_to_strict = 1;
/*
* Make sure there are NO revision (i.e. pending object) parameter,
* rev.max_count is reasonable (0 <= n <= 3), and
* there is no other revision filtering parameters.
*/
if (rev.pending.nr ||
rev.min_age != -1 || rev.max_age != -1 ||
3 < rev.max_count)
usage(diff_files_usage);
/*
* "diff-files --base -p" should not combine merges because it
* was not asked to. "diff-files -c -p" should not densify
* (the user should ask with "diff-files --cc" explicitly).
*/
if (rev.max_count == -1 &&
(rev.diffopt.output_format & DIFF_FORMAT_PATCH))
diff_merges_set_dense_combined_if_unset(&rev);
if (read_cache_preload(&rev.diffopt.pathspec) < 0) {
perror("read_cache_preload");
result = -1;
goto cleanup;
}
result = run_diff_files(&rev, options);
result = diff_result_code(&rev.diffopt, result);
cleanup:
release_revisions(&rev);
return result;
}