| Message ID | 20190912221240.18057-4-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some git clean issues | expand |
Elijah Newren <newren@gmail.com> writes: > For a pathspec like 'foo/bar' comparing against a path named "foo/", > namelen will be 4, and match[namelen] will be 'b'. The correct location > of the directory separator is namelen-1. And the reason why name[namelen-1] may not be slash, in which case your new code makes offset 0, is because we need to handle what case? When path is "foo" (not "foo/")? Just makes me wonder why this callee allows the caller(s) to be inconsistent, sometimes including the trailing slash in <name, nemelen> tuple, sometimes not. > The reason the code worked anyway was that the following code immediately > checked whether the first matchlen characters matched (which they do) and > then bailed and return MATCHED_RECURSIVELY anyway since wildmatch doesn't > have the ability to check if "name" can be matched as a directory (or > prefix) against the pathspec. Nicely spotted and explained. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > dir.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/dir.c b/dir.c > index a9168bed96..bf1a74799e 100644 > --- a/dir.c > +++ b/dir.c > @@ -356,8 +356,9 @@ static int match_pathspec_item(const struct index_state *istate, > /* Perform checks to see if "name" is a super set of the pathspec */ > if (flags & DO_MATCH_SUBMODULE) { > /* name is a literal prefix of the pathspec */ > + int offset = name[namelen-1] == '/' ? 1 : 0; > if ((namelen < matchlen) && > - (match[namelen] == '/') && > + (match[namelen-offset] == '/') && > !ps_strncmp(item, match, name, namelen)) > return MATCHED_RECURSIVELY;
diff --git a/dir.c b/dir.c index a9168bed96..bf1a74799e 100644 --- a/dir.c +++ b/dir.c @@ -356,8 +356,9 @@ static int match_pathspec_item(const struct index_state *istate, /* Perform checks to see if "name" is a super set of the pathspec */ if (flags & DO_MATCH_SUBMODULE) { /* name is a literal prefix of the pathspec */ + int offset = name[namelen-1] == '/' ? 1 : 0; if ((namelen < matchlen) && - (match[namelen] == '/') && + (match[namelen-offset] == '/') && !ps_strncmp(item, match, name, namelen)) return MATCHED_RECURSIVELY;
For a pathspec like 'foo/bar' comparing against a path named "foo/", namelen will be 4, and match[namelen] will be 'b'. The correct location of the directory separator is namelen-1. The reason the code worked anyway was that the following code immediately checked whether the first matchlen characters matched (which they do) and then bailed and return MATCHED_RECURSIVELY anyway since wildmatch doesn't have the ability to check if "name" can be matched as a directory (or prefix) against the pathspec. Signed-off-by: Elijah Newren <newren@gmail.com> --- dir.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)