{"thread":{"id":"19431","subject":"[PATCH] apply: handle filenames with double slashes better","startedAt":"2009-05-21T12:25:11Z","lastAt":"2009-05-25T09:11:43Z","messageCount":5,"participants":["Michal Marek","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"114409","messageId":"20090521122511.GA31614@sepie.suse.cz","threadId":"19431","inReplyTo":null,"subject":"[PATCH] apply: handle filenames with double slashes better","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-05-21T12:25:11Z","receivedAt":"2009-05-21T12:25:11Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"Collapse double slashes to make patches like this work with --index or\n--cached:\n\ngit apply --index <<-EOF\n\t--- a/perl//Git.pm\n\t+++ b/perl//Git.pm\n\t@@ -1358,3 +1358,4 @@\n\n\n\t 1; # Famous last words\n\t+# test\nEOF\n\nSigned-off-by: Michal Marek <mmarek@suse.cz>\n---\n builtin-apply.c |   26 ++++++++++++++++++++------\n 1 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8a3771e..0c623e4 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -320,6 +320,20 @@ static int name_terminate(const char *name, int namelen, int c, int terminate)\n \treturn 1;\n }\n \n+/* remove double slashes to make --index work with such filenames */\n+static char *canon_name(char *name)\n+{\n+\tint i = 0, j = 0;\n+\n+\twhile (name[i]) {\n+\t\tif ((name[j++] = name[i++]) == '/')\n+\t\t\twhile (name[i] == '/')\n+\t\t\t\ti++;\n+\t}\n+\tname[j] = '\\0';\n+\treturn name;\n+}\n+\n static char *find_name(const char *line, char *def, int p_value, int terminate)\n {\n \tint len;\n@@ -349,7 +363,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\t\t\tfree(def);\n \t\t\t\tif (root)\n \t\t\t\t\tstrbuf_insert(&name, 0, root, root_len);\n-\t\t\t\treturn strbuf_detach(&name, NULL);\n+\t\t\t\treturn canon_name(strbuf_detach(&name, NULL));\n \t\t\t}\n \t\t}\n \t\tstrbuf_release(&name);\n@@ -369,10 +383,10 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\t\tstart = line;\n \t}\n \tif (!start)\n-\t\treturn def;\n+\t\treturn canon_name(def);\n \tlen = line - start;\n \tif (!len)\n-\t\treturn def;\n+\t\treturn canon_name(def);\n \n \t/*\n \t * Generally we prefer the shorter name, especially\n@@ -383,7 +397,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \tif (def) {\n \t\tint deflen = strlen(def);\n \t\tif (deflen < len && !strncmp(start, def, deflen))\n-\t\t\treturn def;\n+\t\t\treturn canon_name(def);\n \t\tfree(def);\n \t}\n \n@@ -392,10 +406,10 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\tstrcpy(ret, root);\n \t\tmemcpy(ret + root_len, start, len);\n \t\tret[root_len + len] = '\\0';\n-\t\treturn ret;\n+\t\treturn canon_name(ret);\n \t}\n \n-\treturn xmemdupz(start, len);\n+\treturn canon_name(xmemdupz(start, len));\n }\n \n static int count_slashes(const char *cp)\n-- \n1.6.3\n"},{"id":"114424","messageId":"7vd4a2bj3p.fsf@alter.siamese.dyndns.org","threadId":"19431","inReplyTo":"20090521122511.GA31614@sepie.suse.cz","subject":"Re: [PATCH] apply: handle filenames with double slashes better","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-21T14:56:10Z","receivedAt":"2009-05-21T14:56:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Marek <mmarek@suse.cz> writes:\n\n> Collapse double slashes to make patches like this work with --index or\n> --cached:\n>\n> git apply --index <<-EOF\n> \t--- a/perl//Git.pm\n> \t+++ b/perl//Git.pm\n> \t@@ -1358,3 +1358,4 @@\n>\n>\n> \t 1; # Famous last words\n> \t+# test\n> EOF\n>\n> Signed-off-by: Michal Marek <mmarek@suse.cz>\n\nHmm, I do not know if this is a good change.\n\nFor duplicate slashes in paths, I do not think there is any other sensible\nway to handle them other than squashing them together, but naming the\nfunction to do so \"canon_name()\" would tempt people to add other\nnot-so-clearly-sensible \"canonicalization\" such as turning \"./a\" to \"a\"\n(which we shouldn't --- we should treat \"./\" as one level so that we keep\nbehaving in a similar way as \"patch -p1\" does) or \"a/../b\" to \"b\".\n\nAlso calling this in find_name() loses information too early in the\nprocessing; how bad would it look if you move the callsite of this\nduplicate slash squashing down the callchain where the names are actually\nused?\n"},{"id":"114438","messageId":"20090521191204.GA29362@sepie.suse.cz","threadId":"19431","inReplyTo":"7vd4a2bj3p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] apply: handle filenames with double slashes better","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-05-21T19:12:04Z","receivedAt":"2009-05-21T19:12:04Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"On Thu, May 21, 2009 at 07:56:10AM -0700, Junio C Hamano wrote:\n> Michal Marek <mmarek@suse.cz> writes:\n> \n> > Collapse double slashes to make patches like this work with --index or\n> > --cached:\n> >\n> > git apply --index <<-EOF\n> > \t--- a/perl//Git.pm\n> > \t+++ b/perl//Git.pm\n> > \t@@ -1358,3 +1358,4 @@\n> >\n> >\n> > \t 1; # Famous last words\n> > \t+# test\n> > EOF\n> >\n> > Signed-off-by: Michal Marek <mmarek@suse.cz>\n> \n> Hmm, I do not know if this is a good change.\n> \n> For duplicate slashes in paths, I do not think there is any other sensible\n> way to handle them other than squashing them together, but naming the\n> function to do so \"canon_name()\" would tempt people to add other\n> not-so-clearly-sensible \"canonicalization\" such as turning \"./a\" to \"a\"\n> (which we shouldn't --- we should treat \"./\" as one level so that we keep\n> behaving in a similar way as \"patch -p1\" does) or \"a/../b\" to \"b\".\n\nOK, renamed to squash_slash().\n\n\n> Also calling this in find_name() loses information too early in the\n> processing; how bad would it look if you move the callsite of this\n> duplicate slash squashing down the callchain where the names are actually\n> used?\n\nI tried this, but I'm not sure it's better now (and there might be some\ninconsistencies left). IMHO removing the double slashes is an operation\nsimilar to the unquoting done in find_name(), i.e. converting the text\nin the patch to something that can be passed to lstat() or\ncache_name_lookup(). Anyway, new patch attached.\n\nMichal\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8a3771e..5a35d9f 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -320,6 +320,22 @@ static int name_terminate(const char *name, int namelen, int c, int terminate)\n \treturn 1;\n }\n \n+/* remove double slashes to make --index work with such filenames */\n+static char *squash_slash(char *name)\n+{\n+\tint i = 0, j = 0;\n+\n+\tif (!name)\n+\t\treturn name;\n+\twhile (name[i]) {\n+\t\tif ((name[j++] = name[i++]) == '/')\n+\t\t\twhile (name[i] == '/')\n+\t\t\t\ti++;\n+\t}\n+\tname[j] = '\\0';\n+\treturn name;\n+}\n+\n static char *find_name(const char *line, char *def, int p_value, int terminate)\n {\n \tint len;\n@@ -423,6 +439,7 @@ static int guess_p_value(const char *nameline)\n \tname = find_name(nameline, NULL, 0, TERM_SPACE | TERM_TAB);\n \tif (!name)\n \t\treturn -1;\n+\tname = squash_slash(name);\n \tcp = strchr(name, '/');\n \tif (!cp)\n \t\tval = 0;\n@@ -2416,7 +2433,7 @@ static int verify_index_match(struct cache_entry *ce, struct stat *st)\n \n static int check_preimage(struct patch *patch, struct cache_entry **ce, struct stat *st)\n {\n-\tconst char *old_name = patch->old_name;\n+\tconst char *old_name = squash_slash(patch->old_name);\n \tstruct patch *tpatch = NULL;\n \tint stat_ret = 0;\n \tunsigned st_mode = 0;\n@@ -2503,8 +2520,8 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s\n static int check_patch(struct patch *patch)\n {\n \tstruct stat st;\n-\tconst char *old_name = patch->old_name;\n-\tconst char *new_name = patch->new_name;\n+\tconst char *old_name = squash_slash(patch->old_name);\n+\tconst char *new_name = squash_slash(patch->new_name);\n \tconst char *name = old_name ? old_name : new_name;\n \tstruct cache_entry *ce = NULL;\n \tstruct patch *tpatch;\n-- \n1.6.3\n"},{"id":"114439","messageId":"7vws8a9s7r.fsf@alter.siamese.dyndns.org","threadId":"19431","inReplyTo":"20090521191204.GA29362@sepie.suse.cz","subject":"Re: [PATCH] apply: handle filenames with double slashes better","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-21T19:22:16Z","receivedAt":"2009-05-21T19:22:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Marek <mmarek@suse.cz> writes:\n\n> I tried this, but I'm not sure it's better now (and there might be some\n> inconsistencies left). IMHO removing the double slashes is an operation\n> similar to the unquoting done in find_name(), i.e. converting the text\n> in the patch to something that can be passed to lstat() or\n> cache_name_lookup().\n\nOk, you convinced me.\n\nLet's take your first patch with just the s/canon_name/squash_slash/\nchange.\n\nThanks.\n"},{"id":"114639","messageId":"20090525091142.GA17042@sepie.suse.cz","threadId":"19431","inReplyTo":"7vws8a9s7r.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] apply: handle filenames with double slashes better","fromName":"Michal Marek","fromEmail":"mmarek@suse.cz","sentAt":"2009-05-25T09:11:43Z","receivedAt":"2009-05-25T09:11:43Z","isPatch":true,"sender":{"key":"mmarek@suse.cz","avatar":"https://gravatar.com/avatar/b026025af60a27d7ea0635833f351de6b0a6089a9573415d86c9352eda4f6e9a?d=mp&s=160"},"body":"On Thu, May 21, 2009 at 12:22:16PM -0700, Junio C Hamano wrote:\n> Let's take your first patch with just the s/canon_name/squash_slash/\n> change.\n\nSorry for the delay, Here's hopefully the final patch:\n\nCollapse double slashes to make patches like this work with --index or\n--cached:\n\ngit apply --index <<-EOF\n\t--- a/perl//Git.pm\n\t+++ b/perl//Git.pm\n\t@@ -1358,3 +1358,4 @@\n\n\t 1; # Famous last words\n\t+# test\nEOF\n\nSigned-off-by: Michal Marek <mmarek@suse.cz>\n---\n builtin-apply.c |   28 ++++++++++++++++++++++------\n 1 files changed, 22 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-apply.c b/builtin-apply.c\nindex 8a3771e..10fefa0 100644\n--- a/builtin-apply.c\n+++ b/builtin-apply.c\n@@ -320,6 +320,22 @@ static int name_terminate(const char *name, int namelen, int c, int terminate)\n \treturn 1;\n }\n \n+/* remove double slashes to make --index work with such filenames */\n+static char *squash_slash(char *name)\n+{\n+\tint i = 0, j = 0;\n+\n+\tif (!name)\n+\t\treturn name;\n+\twhile (name[i]) {\n+\t\tif ((name[j++] = name[i++]) == '/')\n+\t\t\twhile (name[i] == '/')\n+\t\t\t\ti++;\n+\t}\n+\tname[j] = '\\0';\n+\treturn name;\n+}\n+\n static char *find_name(const char *line, char *def, int p_value, int terminate)\n {\n \tint len;\n@@ -349,7 +365,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\t\t\tfree(def);\n \t\t\t\tif (root)\n \t\t\t\t\tstrbuf_insert(&name, 0, root, root_len);\n-\t\t\t\treturn strbuf_detach(&name, NULL);\n+\t\t\t\treturn squash_slash(strbuf_detach(&name, NULL));\n \t\t\t}\n \t\t}\n \t\tstrbuf_release(&name);\n@@ -369,10 +385,10 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\t\tstart = line;\n \t}\n \tif (!start)\n-\t\treturn def;\n+\t\treturn squash_slash(def);\n \tlen = line - start;\n \tif (!len)\n-\t\treturn def;\n+\t\treturn squash_slash(def);\n \n \t/*\n \t * Generally we prefer the shorter name, especially\n@@ -383,7 +399,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \tif (def) {\n \t\tint deflen = strlen(def);\n \t\tif (deflen < len && !strncmp(start, def, deflen))\n-\t\t\treturn def;\n+\t\t\treturn squash_slash(def);\n \t\tfree(def);\n \t}\n \n@@ -392,10 +408,10 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)\n \t\tstrcpy(ret, root);\n \t\tmemcpy(ret + root_len, start, len);\n \t\tret[root_len + len] = '\\0';\n-\t\treturn ret;\n+\t\treturn squash_slash(ret);\n \t}\n \n-\treturn xmemdupz(start, len);\n+\treturn squash_slash(xmemdupz(start, len));\n }\n \n static int count_slashes(const char *cp)\n-- \n1.6.3\n"}]}