@matheusbsbs / matheus-bagliano / commits / 5304810044

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

Jeff King committed May 14, 2014 at 18:13 UTC 530481004443a00fbf5ab477b36b90508b4dc59d

2 files changed +32 -12

diff-lib.c

+21 -12

@@ -99,7 +99,6 @@ int run_diff_files(struct rev_info *revs, unsigned int option)

99 diff_unmerged_stage = 2;

100 entries = active_nr;

101 for (i = 0; i < entries; i++) {

102 - struct stat st;

102 unsigned int oldmode, newmode;

103 struct cache_entry *ce = active_cache[i];

104 int changed;

@@ -117,6 +116,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)

116 unsigned int wt_mode = 0;

117 int num_compare_stages = 0;

118 size_t path_len;

119 + struct stat st;

120

121 path_len = ce_namelen(ce);

122

@@ -198,26 +198,35 @@ int run_diff_files(struct rev_info *revs, unsigned int option)

198 continue;

199

200 /* If CE_VALID is set, don't look at workdir for file removal */

201 - changed = (ce->ce_flags & CE_VALID) ? 0 : check_removed(ce, &st);

202 - if (changed) {

203 - if (changed < 0) {

204 - perror(ce->name);

201 + if (ce->ce_flags & CE_VALID) {

202 + changed = 0;

203 + newmode = ce->ce_mode;

204 + } else {

205 + struct stat st;

206 +

207 + changed = check_removed(ce, &st);

208 + if (changed) {

209 + if (changed < 0) {

210 + perror(ce->name);

211 + continue;

212 + }

213 + diff_addremove(&revs->diffopt, '-', ce->ce_mode,

214 + ce->sha1, !is_null_sha1(ce->sha1),

215 + ce->name, 0);

216 continue;

217 }

207 - diff_addremove(&revs->diffopt, '-', ce->ce_mode,

208 - ce->sha1, !is_null_sha1(ce->sha1),

209 - ce->name, 0);

210 - continue;

218 +

219 + changed = match_stat_with_submodule(&revs->diffopt, ce, &st,

220 + ce_option, &dirty_submodule);

221 + newmode = ce_mode_from_stat(ce, st.st_mode);

222 }

212 - changed = match_stat_with_submodule(&revs->diffopt, ce, &st,

213 - ce_option, &dirty_submodule);

223 +

224 if (!changed && !dirty_submodule) {

225 ce_mark_uptodate(ce);

226 if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))

227 continue;

228 }

229 oldmode = ce->ce_mode;

220 - newmode = ce_mode_from_stat(ce, st.st_mode);

230 diff_change(&revs->diffopt, oldmode, newmode,

231 ce->sha1, (changed ? null_sha1 : ce->sha1),

232 !is_null_sha1(ce->sha1), (changed ? 0 : !is_null_sha1(ce->sha1)),

t/t4039-diff-assume-unchanged.sh

+11

@@ -28,4 +28,15 @@ test_expect_success 'diff-files does not examine assume-unchanged entries' '

28 test -z "$(git diff-files -- one)"

29 '

31 +test_expect_success POSIXPERM 'find-copies-harder is not confused by mode bits' '

32 + echo content >exec &&

33 + chmod +x exec &&

34 + git add exec &&

35 + git commit -m exec &&

36 + git update-index --assume-unchanged exec &&

37 + >expect &&

38 + git diff-files --find-copies-harder -- exec >actual &&

39 + test_cmp expect actual

40 +'

41 +

42 test_done