Re: [PATCH 17/21] list-files: show directories as well as files

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2015, 21:51 UTC
Message-ID: <xmqqmw53ki3i.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1422189476-7518-18-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 30 quoted lines

> @@ -194,16 +225,31 @@ static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce)
>  static void show_ce_entry(const char *tag, const struct cache_entry *ce)
>  {
>  	static struct strbuf sb = STRBUF_INIT;
> -	int len = max_prefix_len;
> +	int len = max_prefix_len, saved_max_depth;
>  
>  	if (len >= ce_namelen(ce))
>  		die("git ls-files: internal error - cache entry not superset of prefix");
>  
> +	if (show_dirs) {
> +		/* ignore depth to catch dirs that contain matched entries */
> +		saved_max_depth = pathspec.max_depth;
> +		pathspec.max_depth = -1;
> +	}
> +
>  	if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
>  			    len, ps_matched,
>  			    S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
>  		return;
>  
> +	if (show_dirs) {
> +		pathspec.max_depth = saved_max_depth;
> +		if (strchr(ce->name, '/') &&
> +		    !match_pathspec(&pathspec, ce->name, ce_namelen(ce),
> +				    prefix_len, NULL, 1) &&
> +		    show_as_directory(ce))
> +			return;
> +	}
> +

My compiler seems to be too stupid to notice that saved_max_depth is always set before it is used, if it gets used and complains. Sigh.

For now I am tempted to squash this in. Note that the original does not seem to restore saved_max_depath when the pathspec does not match and function returns in the call to match_pathspec() we have in the code before your patch, which smells like a bug, and the attached would fix it.

 builtin/ls-files.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 29b5c2e..f28b7e9 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -222,27 +222,38 @@ static void write_ce_name(struct strbuf *sb, const struct cache_entry *ce)
 	strbuf_release(&quoted);
 }
 
+static int match_pathspec_with_depth(struct pathspec *ps,
+				     const char *name, int namelen,
+				     int prefix, char *seen, int is_dir,
+				     const int *custom_depth)
+{
+	int saved_depth = ps->max_depth;
+	int result;
+
+	if (custom_depth)
+		ps->max_depth = *custom_depth;
+	result = match_pathspec(ps, name, namelen, prefix, seen, is_dir);
+	if (custom_depth)
+		ps->max_depth = saved_depth;
+	return result;
+}
+
 static void show_ce_entry(const char *tag, const struct cache_entry *ce)
 {
 	static struct strbuf sb = STRBUF_INIT;
-	int len = max_prefix_len, saved_max_depth;
+	int len = max_prefix_len;
+	static const int infinite_depth = -1;
 
 	if (len >= ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
-	if (show_dirs) {
-		/* ignore depth to catch dirs that contain matched entries */
-		saved_max_depth = pathspec.max_depth;
-		pathspec.max_depth = -1;
-	}
-
-	if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
-			    len, ps_matched,
-			    S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
+	if (!match_pathspec_with_depth(&pathspec, ce->name, ce_namelen(ce),
+				       len, ps_matched,
+				       S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode),
+				       show_dirs ? &infinite_depth : NULL))
 		return;
 
 	if (show_dirs) {
-		pathspec.max_depth = saved_max_depth;
 		if (strchr(ce->name, '/') &&
 		    !match_pathspec(&pathspec, ce->name, ce_namelen(ce),
 				    prefix_len, NULL, 1) &&