Skip to content

Commit 902b90c

Browse files
newrengitster
authored andcommitted
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 396049e ("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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ca8b539 commit 902b90c

File tree

1 file changed

+1
-1
lines changed

1 file changed

+1
-1
lines changed

builtin/clean.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
10181018
for_each_string_list_item(item, &del_list) {
10191019
struct stat st;
10201020

1021+
strbuf_reset(&abs_path);
10211022
if (prefix)
10221023
strbuf_addstr(&abs_path, prefix);
10231024

@@ -1051,7 +1052,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
10511052
printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname);
10521053
}
10531054
}
1054-
strbuf_reset(&abs_path);
10551055
}
10561056

10571057
strbuf_release(&abs_path);

0 commit comments

Comments
 (0)