diff options
author | Elijah Newren <newren@gmail.com> | 2019-09-17 09:35:04 -0700 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-09-17 12:20:36 -0700 |
commit | 902b90cf42bc3c05f9fd124edac42ef1fc4ffafa (patch) | |
tree | 57e83312d6434e21d1fca863aac3b99e98ef052d /builtin/rm.c | |
parent | clean: rewrap overly long line (diff) | |
download | tgif-902b90cf42bc3c05f9fd124edac42ef1fc4ffafa.tar.xz |
clean: fix theoretical path corruption
cmd_clean() had the following code structure:
struct strbuf abs_path = STRBUF_INIT;
for_each_string_list_item(item, &del_list) {
strbuf_addstr(&abs_path, prefix);
strbuf_addstr(&abs_path, item->string);
PROCESS(&abs_path);
strbuf_reset(&abs_path);
}
where I've elided a bunch of unnecessary details and PROCESS(&abs_path)
represents a big chunk of code rather than an actual function call. One
piece of PROCESS was:
if (lstat(abs_path.buf, &st))
continue;
which would cause the strbuf_reset() to be missed -- meaning that the
next path to be handled would have two paths concatenated. This path
used to use die_errno() instead of continue prior to commit 396049e5fb62
("git-clean: refactor git-clean into two phases", 2013-06-25), but my
understanding of how correct_untracked_entries() works is that it will
prevent both dir/ and dir/file from being in the list to clean so this
should be dead code and the die_errno() should be safe. But I hesitate
to remove it since I am not certain.
However, we can fix both this bug and possible similar future bugs by
simply moving the strbuf_reset(&abs_path) to the beginning of the loop.
It'll result in N calls to strbuf_reset() instead of N-1, but that's a
small price to pay to avoid sneaky bugs like this.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'builtin/rm.c')
0 files changed, 0 insertions, 0 deletions