Re: [PATCH] apply: handle filenames with double slashes better

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] apply: handle filenames with double slashes better

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:46:48

Michal Marek [off-list ref] writes:
Collapse double slashes to make patches like this work with --index or
--cached:

git apply --index <<-EOF
	--- a/perl//Git.pm
	+++ b/perl//Git.pm
	@@ -1358,3 +1358,4 @@


	 1; # Famous last words
	+# test
EOF

Signed-off-by: Michal Marek <redacted>
Hmm, I do not know if this is a good change.

For duplicate slashes in paths, I do not think there is any other sensible
way to handle them other than squashing them together, but naming the
function to do so "canon_name()" would tempt people to add other
not-so-clearly-sensible "canonicalization" such as turning "./a" to "a"
(which we shouldn't --- we should treat "./" as one level so that we keep
behaving in a similar way as "patch -p1" does) or "a/../b" to "b".

Also calling this in find_name() loses information too early in the
processing; how bad would it look if you move the callsite of this
duplicate slash squashing down the callchain where the names are actually
used?

Re: [PATCH] apply: handle filenames with double slashes better

From: Michal Marek <hidden>
Date: 2016-06-15 22:46:48

On Thu, May 21, 2009 at 07:56:10AM -0700, Junio C Hamano wrote:
Michal Marek [off-list ref] writes:
quoted
Collapse double slashes to make patches like this work with --index or
--cached:

git apply --index <<-EOF
	--- a/perl//Git.pm
	+++ b/perl//Git.pm
	@@ -1358,3 +1358,4 @@


	 1; # Famous last words
	+# test
EOF

Signed-off-by: Michal Marek <redacted>
Hmm, I do not know if this is a good change.

For duplicate slashes in paths, I do not think there is any other sensible
way to handle them other than squashing them together, but naming the
function to do so "canon_name()" would tempt people to add other
not-so-clearly-sensible "canonicalization" such as turning "./a" to "a"
(which we shouldn't --- we should treat "./" as one level so that we keep
behaving in a similar way as "patch -p1" does) or "a/../b" to "b".
OK, renamed to squash_slash().

Also calling this in find_name() loses information too early in the
processing; how bad would it look if you move the callsite of this
duplicate slash squashing down the callchain where the names are actually
used?
I tried this, but I'm not sure it's better now (and there might be some
inconsistencies left). IMHO removing the double slashes is an operation
similar to the unquoting done in find_name(), i.e. converting the text
in the patch to something that can be passed to lstat() or
cache_name_lookup(). Anyway, new patch attached.

Michal
diff --git a/builtin-apply.c b/builtin-apply.c
index 8a3771e..5a35d9f 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -320,6 +320,22 @@ static int name_terminate(const char *name, int namelen, int c, int terminate)
 	return 1;
 }
 
+/* remove double slashes to make --index work with such filenames */
+static char *squash_slash(char *name)
+{
+	int i = 0, j = 0;
+
+	if (!name)
+		return name;
+	while (name[i]) {
+		if ((name[j++] = name[i++]) == '/')
+			while (name[i] == '/')
+				i++;
+	}
+	name[j] = '\0';
+	return name;
+}
+
 static char *find_name(const char *line, char *def, int p_value, int terminate)
 {
 	int len;
@@ -423,6 +439,7 @@ static int guess_p_value(const char *nameline)
 	name = find_name(nameline, NULL, 0, TERM_SPACE | TERM_TAB);
 	if (!name)
 		return -1;
+	name = squash_slash(name);
 	cp = strchr(name, '/');
 	if (!cp)
 		val = 0;
@@ -2416,7 +2433,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st)
 
 static int check_preimage(struct patch *patch, struct cache_entry **ce, struct stat *st)
 {
-	const char *old_name = patch->old_name;
+	const char *old_name = squash_slash(patch->old_name);
 	struct patch *tpatch = NULL;
 	int stat_ret = 0;
 	unsigned st_mode = 0;
@@ -2503,8 +2520,8 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
 static int check_patch(struct patch *patch)
 {
 	struct stat st;
-	const char *old_name = patch->old_name;
-	const char *new_name = patch->new_name;
+	const char *old_name = squash_slash(patch->old_name);
+	const char *new_name = squash_slash(patch->new_name);
 	const char *name = old_name ? old_name : new_name;
 	struct cache_entry *ce = NULL;
 	struct patch *tpatch;
-- 
1.6.3
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help