Re: [PATCH 11/19] tree_entry_interesting: support depth limit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nguyán ThÃi Ngác Duy  <pclouds@xxxxxxxxx> writes:

> diff --git a/dir.c b/dir.c
> index 0987d0c..bb5076c 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -71,6 +71,21 @@ int fill_directory(struct dir_struct *dir, const char **pathspec)
>  	return len;
>  }
>  
> +int within_depth(const char *name, int namelen,
> +			int depth, int max_depth)
> +{
> +	const char *cp = name, *cpe = name + namelen;
> +
> +	while (cp < cpe) {
> +		if (*cp++ != '/')
> +			continue;
> +		depth++;
> +		if (depth > max_depth)
> +			return 0;
> +	}
> +	return 1;
> +}

Makes me almost suspect that it may make more sense to keep track of the
"depth" in a similar way as "base" and "baselen" as "traversal state" on
the side of the caller so that you do not have to scan the string for
slashes over and over again.  But given the codeflow, doing so might make
the result look too ugly, so I won't recommend that without thinking,
though.

> diff --git a/tree-walk.c b/tree-walk.c
> index 40a4657..d28de30 100644
> --- a/tree-walk.c
> +++ b/tree-walk.c
> @@ -558,9 +559,17 @@ int tree_entry_interesting(const struct name_entry *entry,
>  	int pathlen;
>  	int never_interesting = -1;
>  
> -	if (!ps || !ps->nr)
> +	if (!ps)
>  		return 1;
>  
> +	if (!ps->nr) {
> +		if (!ps->recursive || ps->max_depth == -1)
> +			return 1;
> +		return !!within_depth(base, baselen,
> +				      !!S_ISDIR(entry->mode),
> +				      ps->max_depth);
> +	}

This gives different behaviour to between callers that give you NULL as
pathspec and callers that give you a pathspec with zero elements.  Is this
intended?  What is the use case (iow, what does an end user give from the
command line to experience this difference)?

> @@ -571,7 +580,13 @@ int tree_entry_interesting(const struct name_entry *entry,
>  			if (!match_dir_prefix(base, baselen, match, matchlen))
>  				/* Just a random prefix match */
>  				continue;
> -			return 2;
> +
> +			if (!ps->recursive || ps->max_depth == -1)
> +				return 2;
> +
> +			return !!within_depth(base+matchlen+1, baselen-matchlen-1,
> +					      !!S_ISDIR(entry->mode),
> +					      ps->max_depth);
>  		}

If two pathspecs that overlap with each other (e.g. "Documentation/" and
"Documentation/technical") are given, and if the shorter one comes before
the longer one in ps[], wouldn't this give you an unexpected result?  When
inspecting "Documentation/technical/api/foo.txt" with depth limit of 2, if
you didn't have "Documentation/" pathspec, you count "api/foo.txt"
relative to "Documentation/technical", declare that the path is within
limit, and show it.  But if you have "Documentation/" in ps[], you look at
it, decide "technical/api/foo.txt" is too deep and return false without
even looking at "Documentation/technical" that may appear later in ps[],
no?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]