threads / patch / 22251

patchbuiltin-apply.c: Skip filenames without enough components

Subject: [PATCH] builtin-apply.c: Skip filenames without enough components

## tl;dr

5 messages between Jan 17, 2010 and Jan 18, 2010. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Andreas Gruenbacher· Jan 17, 2010, 02:05 UTC · lore

find_name() wrongly returned the whole filename for filenames without enough leading pathname components (e.g., when applying a patch to a top-level file with -p2).

Include the -p value used in the error message when no filenames can be found.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 builtin-apply.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)
Show changes to builtin-apply.c +9 −2
diff --git a/builtin-apply.c b/builtin-apply.c
index 541493e..b99db0b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -404,6 +404,9 @@ static char *squash_slash(char *name)
 {
 	int i = 0, j = 0;
 
+	if (!name)
+		return NULL;
+
 	while (name[i]) {
 		if ((name[j++] = name[i++]) == '/')
 			while (name[i] == '/')
@@ -416,7 +419,10 @@ static char *squash_slash(char *name)
 static char *find_name(const char *line, char *def, int p_value, int terminate)
 {
 	int len;
-	const char *start = line;
+	const char *start = NULL;
+
+	if (p_value == 0)
+		start = line;
 
 	if (*line == '"') {
 		struct strbuf name = STRBUF_INIT;
@@ -1199,7 +1205,8 @@ static int find_header(char *line, unsigned long size, int *hdrsize, 
struct patc
 				continue;
 			if (!patch->old_name && !patch->new_name) {
 				if (!patch->def_name)
-					die("git diff header lacks filename information (line %d)", linenr);
+					die("git diff header lacks filename information when removing "
+					    "%d leading pathname components (line %d)" , p_value, linenr);
 				patch->old_name = patch->new_name = patch->def_name;
 			}
 			patch->is_toplevel_relative = 1;
-- 
1.6.6.197.g9c4a28
Andreas Gruenbacher· Jan 17, 2010, 02:44 UTC · re: Junio C Hamano · lore

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:
> Tests?

Sure if you think it's worth a regression test ... "git apply -p2" of the following patch fails with "fatal: git diff header lacks filename information when removing 2 leading pathname components (line 6)" with the fix, and creates b/f without:

	diff --git a/f b/f
	new file mode 100644
	index 0000000..6a69f92
	--- /dev/null
	+++ b/f
	@@ -0,0 +1 @@
	+f

(Some earlier versions of git failed with "fatal: git apply: bad git-diff - inconsistent new filename on line 5" in this case.)

Andreas
Nanako Shiraishi· Jan 18, 2010, 10:22 UTC · re: Andreas Gruenbacher · lore

Re: [PATCH] builtin-apply.c: Skip filenames without enough components

Quoting Andreas Gruenbacher <agruen@suse.de>
> On Sunday 17 January 2010 03:22:10 am Junio C Hamano wrote:
>> Tests?
>
> Sure if you think it's worth a regression test ...
Of course you must have a test when you are fixing things. Tests aren't to prove that your patch is correct. They are to prevent other people from breaking your change long after you leave the project.
> Sure if you think it's worth a regression test ... "git apply -p2" of the 
> following patch fails...
You do so by sending a patch to add your new test in t/; adding to an existing related test is preferred if the test is small.
Junio, in case you don't want to wait for Andreas, you can squash this test in.
Show changes to t/t4120-apply-popt.sh +5 −0
diff --git a/t/t4120-apply-popt.sh b/t/t4120-apply-popt.sh
index 83d4ba6..b463b4f 100755
--- a/t/t4120-apply-popt.sh
+++ b/t/t4120-apply-popt.sh
@@ -22,4 +22,9 @@ test_expect_success 'apply git diff with -p2' '
 	git apply -p2 patch.file
 '
 
+test_expect_success 'apply with too large -p' '
+	test_must_fail git apply --stat -p3 patch.file 2>err &&
+	grep "removing 3 leading" err
+'
+
 test_done
-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

← back to recent threads