[v3,03/12] dir: fix off-by-one error in match_pathspec_item
diff mbox series

Message ID 20190912221240.18057-4-newren@gmail.com
State New
Headers show
Series
  • Fix some git clean issues
Related show

Commit Message

Elijah Newren Sept. 12, 2019, 10:12 p.m. UTC
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(-)

Comments

Junio C Hamano Sept. 13, 2019, 7:05 p.m. UTC | #1
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;

Patch
diff mbox series

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;