From: Junio C Hamano Date: Fri, 22 Jan 2010 01:40:41 GMT Subject: Re: [PATCH] Handle double slashes in make_relative_path() Message-ID: <7vpr52gbmu.fsf@alter.siamese.dyndns.org> In-Reply-To: <379d55c6a4110736aadb8ace3b050de879a9deab.1264118830.git.trast@student.ethz.ch> Thomas Rast writes: > diff --git a/path.c b/path.c > index 2ec950b..a195bab 100644 > --- a/path.c > +++ b/path.c > @@ -400,10 +400,10 @@ int set_shared_perm(const char *path, int mode) > baselen = strlen(base); > if (prefixcmp(abs, base)) > return abs; > - if (abs[baselen] == '/') > - baselen++; > - else if (base[baselen - 1] != '/') > + if (abs[baselen] != '/' && base[baselen - 1] != '/') > return abs; > + while (abs[baselen] == '/') > + baselen++; > strcpy(buf, abs + baselen); > return buf; > } Curious; why does your hunk header says set_shared_perm() while this is a patch to make_relative_path()? Do you run a broken git with funny funcname regexp pattern? The function takes two paths, an early part of abs is supposed to match base; otherwise abs is not a path under base and the function returns the full path of abs. Now what is the goal of this patch? To allow people to have duplicated slashes at random places in either abs or base, or is it only interested in a particular input that is malformed? If the latter, what is the permitted non-canonical input? If abs were "/a//b/c" and base were "/a/b", then the combination is rejected by prefixcmp() and full "/a//b/c" is returned. Is it the intended behaviour of the patch? I would actually have expected to see something like this, but I haven't even compile tested it, so... path.c | 29 ++++++++++++++++++++--------- 1 files changed, 20 insertions(+), 9 deletions(-) diff --git a/path.c b/path.c index 2ec950b..1c3570c 100644 --- a/path.c +++ b/path.c @@ -394,17 +394,28 @@ int set_shared_perm(const char *path, int mode) const char *make_relative_path(const char *abs, const char *base) { static char buf[PATH_MAX + 1]; - int baselen; + int i = 0, j = 0; + if (!base) return abs; - baselen = strlen(base); - if (prefixcmp(abs, base)) - return abs; - if (abs[baselen] == '/') - baselen++; - else if (base[baselen - 1] != '/') - return abs; - strcpy(buf, abs + baselen); + while (base[i]) { + if (base[i] == '/') { + if (abs[j] != '/') + return abs; + while (base[i] == '/') + i++; + while (abs[j] == '/') + j++; + continue; + } else if (abs[j] != base[i]) { + return abs; + } + i++; + j++; + } + while (abs[j] == '/') + j++; + strcpy(buf, abs + j); return buf; }