[PATCH v3 0/6] cache_tree_find() workover

[PATCH v3 0/6] cache_tree_find() workover

[Michael Haggerty]

Who would have expected so much slop in one little function?  Here are
the changes coming out of a long email thread.

I feel a little bit silly submitting six separate patches, but they
really are independent.  And two of the changes were suggested by
other people, so splitting those out, at least, helps give credit
where it is due.  But feel free to squash the patches together if you
think it is best.

Changes since v2 (aside from the atomization):

* Use strchrnul() to initialize slash (as suggested by Junio)

* Don't initialize variable at declaration (as per Junio's
  preference).

Thanks to Junio and David for this workout.

Michael Haggerty (6):
  cache_tree_find(): remove redundant checks
  cache_tree_find(): initialize slash using strchrnul()
  cache_tree_find(): fix comment formatting
  cache_tree_find(): remove redundant check
  cache_tree_find(): remove early return
  cache_tree_find(): use path variable when passing over slashes

 cache-tree.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

-- 
1.9.0

[PATCH v3 1/6] cache_tree_find(): remove redundant checks

[Michael Haggerty]

slash is initialized to a value that cannot be NULL.  So remove the
guards against slash == NULL later in the loop.

Suggested-by: David Kastrup <dak@gnu.org>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..4d439bd 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -564,10 +564,9 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		if (!sub)
 			return NULL;
 		it = sub->cache_tree;
-		if (slash)
-			while (*slash && *slash == '/')
-				slash++;
-		if (!slash || !*slash)
+		while (*slash && *slash == '/')
+			slash++;
+		if (!*slash)
 			return it; /* prefix ended with slashes */
 		path = slash;
 	}
-- 
1.9.0

[PATCH v3 2/6] cache_tree_find(): initialize slash using strchrnul()

[Michael Haggerty]

Suggested-by: Junio Hamano <gitster@pobox.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 4d439bd..d00f4ef 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -554,9 +554,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		const char *slash;
 		struct cache_tree_sub *sub;
 
-		slash = strchr(path, '/');
-		if (!slash)
-			slash = path + strlen(path);
+		slash = strchrnul(path, '/');
 		/* between path and slash is the name of the
 		 * subtree to look for.
 		 */
-- 
1.9.0

[PATCH v3 3/6] cache_tree_find(): fix comment formatting

[Michael Haggerty]

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index d00f4ef..408ee57 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -555,8 +555,9 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		struct cache_tree_sub *sub;
 
 		slash = strchrnul(path, '/');
-		/* between path and slash is the name of the
-		 * subtree to look for.
+		/*
+		 * Between path and slash is the name of the subtree
+		 * to look for.
 		 */
 		sub = find_subtree(it, path, slash - path, 0);
 		if (!sub)
-- 
1.9.0

[PATCH v3 4/6] cache_tree_find(): remove redundant check

[Michael Haggerty]

If *slash == '/', then it is necessarily non-NUL.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache-tree.c b/cache-tree.c
index 408ee57..39ad8c9 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -563,7 +563,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		if (!sub)
 			return NULL;
 		it = sub->cache_tree;
-		while (*slash && *slash == '/')
+		while (*slash == '/')
 			slash++;
 		if (!*slash)
 			return it; /* prefix ended with slashes */
-- 
1.9.0

[PATCH v3 5/6] cache_tree_find(): remove early return

[Michael Haggerty]

There is no need for an early

    return it;

from the loop if slash points at the end of the string, because that
is exactly what will happen when the while condition fails at the
start of the next iteration.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 39ad8c9..17db9f9 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -565,8 +565,6 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		it = sub->cache_tree;
 		while (*slash == '/')
 			slash++;
-		if (!*slash)
-			return it; /* prefix ended with slashes */
 		path = slash;
 	}
 	return it;
-- 
1.9.0

[PATCH v3 6/6] cache_tree_find(): use path variable when passing over slashes

[Michael Haggerty]

The search for the end of the slashes is part of the update of the
path variable for the next iteration as opposed to an update of the
slash variable.  So iterate using path rather than slash.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 17db9f9..7f8d74d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -563,9 +563,10 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
 		if (!sub)
 			return NULL;
 		it = sub->cache_tree;
-		while (*slash == '/')
-			slash++;
+
 		path = slash;
+		while (*path == '/')
+			path++;
 	}
 	return it;
 }
-- 
1.9.0