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 396049e5fb
("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>
			
			
				maint
			
			
		
							parent
							
								
									ca8b5390db
								
							
						
					
					
						commit
						902b90cf42
					
				|  | @ -1018,6 +1018,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix) | ||||||
| 	for_each_string_list_item(item, &del_list) { | 	for_each_string_list_item(item, &del_list) { | ||||||
| 		struct stat st; | 		struct stat st; | ||||||
|  |  | ||||||
|  | 		strbuf_reset(&abs_path); | ||||||
| 		if (prefix) | 		if (prefix) | ||||||
| 			strbuf_addstr(&abs_path, prefix); | 			strbuf_addstr(&abs_path, prefix); | ||||||
|  |  | ||||||
|  | @ -1051,7 +1052,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix) | ||||||
| 				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname); | 				printf(dry_run ? _(msg_would_remove) : _(msg_remove), qname); | ||||||
| 			} | 			} | ||||||
| 		} | 		} | ||||||
| 		strbuf_reset(&abs_path); |  | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
| 	strbuf_release(&abs_path); | 	strbuf_release(&abs_path); | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Elijah Newren
						Elijah Newren