From 530481004443a00fbf5ab477b36b90508b4dc59d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 14 May 2014 18:13:06 -0400 Subject: [PATCH] run_diff_files: do not look at uninitialized stat data If we try to diff an index entry marked CE_VALID (because it was marked with --assume-unchanged), we do not bother even running stat() on the file to see if it was removed. This started long ago with 540e694 (Prevent diff machinery from examining assume-unchanged entries on worktree, 2009-08-11). However, the subsequent code may look at our "struct stat" and expect to find actual data; currently it will find whatever cruft was left on the stack. This can cause problems in two situations: 1. We call match_stat_with_submodule with the stat data, so a submodule may be erroneously marked as changed. 2. If --find-copies-harder is in effect, we pass all entries, even unchanged ones, to diff_change, so it can list them as rename/copy sources. Since we found no change, we assume that function will realize it and not actually display any diff output. However, we end up feeding it a bogus mode, leading it to sometimes claim there was a mode change. We can fix both by splitting the CE_VALID and regular code paths, and making sure only to look at the stat information in the latter. Furthermore, we push the declaration of our "struct stat" down into the code paths that actually set it, so we cannot accidentally access it uninitialized in future code. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff-lib.c | 33 ++++++++++++++++++++------------ t/t4039-diff-assume-unchanged.sh | 11 +++++++++++ 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/diff-lib.c b/diff-lib.c index 346cac651d..73eedca9c8 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -99,7 +99,6 @@ int run_diff_files(struct rev_info *revs, unsigned int option) diff_unmerged_stage = 2; entries = active_nr; for (i = 0; i < entries; i++) { - struct stat st; unsigned int oldmode, newmode; struct cache_entry *ce = active_cache[i]; int changed; @@ -117,6 +116,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option) unsigned int wt_mode = 0; int num_compare_stages = 0; size_t path_len; + struct stat st; path_len = ce_namelen(ce); @@ -198,26 +198,35 @@ int run_diff_files(struct rev_info *revs, unsigned int option) continue; /* If CE_VALID is set, don't look at workdir for file removal */ - changed = (ce->ce_flags & CE_VALID) ? 0 : check_removed(ce, &st); - if (changed) { - if (changed < 0) { - perror(ce->name); + if (ce->ce_flags & CE_VALID) { + changed = 0; + newmode = ce->ce_mode; + } else { + struct stat st; + + changed = check_removed(ce, &st); + if (changed) { + if (changed < 0) { + perror(ce->name); + continue; + } + diff_addremove(&revs->diffopt, '-', ce->ce_mode, + ce->sha1, !is_null_sha1(ce->sha1), + ce->name, 0); continue; } - diff_addremove(&revs->diffopt, '-', ce->ce_mode, - ce->sha1, !is_null_sha1(ce->sha1), - ce->name, 0); - continue; + + changed = match_stat_with_submodule(&revs->diffopt, ce, &st, + ce_option, &dirty_submodule); + newmode = ce_mode_from_stat(ce, st.st_mode); } - changed = match_stat_with_submodule(&revs->diffopt, ce, &st, - ce_option, &dirty_submodule); + if (!changed && !dirty_submodule) { ce_mark_uptodate(ce); if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER)) continue; } oldmode = ce->ce_mode; - newmode = ce_mode_from_stat(ce, st.st_mode); diff_change(&revs->diffopt, oldmode, newmode, ce->sha1, (changed ? null_sha1 : ce->sha1), !is_null_sha1(ce->sha1), (changed ? 0 : !is_null_sha1(ce->sha1)), diff --git a/t/t4039-diff-assume-unchanged.sh b/t/t4039-diff-assume-unchanged.sh index 9d9498bd95..23c0e357a7 100755 --- a/t/t4039-diff-assume-unchanged.sh +++ b/t/t4039-diff-assume-unchanged.sh @@ -28,4 +28,15 @@ test_expect_success 'diff-files does not examine assume-unchanged entries' ' test -z "$(git diff-files -- one)" ' +test_expect_success POSIXPERM 'find-copies-harder is not confused by mode bits' ' + echo content >exec && + chmod +x exec && + git add exec && + git commit -m exec && + git update-index --assume-unchanged exec && + >expect && + git diff-files --find-copies-harder -- exec >actual && + test_cmp expect actual +' + test_done -- 2.47.1