threads / patch / 36066

v3, 6 partscache_tree_find(): remove redundant checks

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

## tl;dr

7 messages between Mar 5, 2014 and Mar 5, 2014. Diffs are folded; open one to read it.

replies: 6people: 1as markdown or json

Michael Haggerty· Mar 5, 2014, 17:26 UTC · lore

[PATCH v3 0/6] cache_tree_find() workover

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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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(-)
Show changes to cache-tree.c +3 −4
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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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

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(-)
Show changes to cache-tree.c +1 −3
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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 cache-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Show changes to cache-tree.c +3 −2
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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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

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(-)
Show changes to cache-tree.c +1 −1
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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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

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(-)
Show changes to cache-tree.c +0 −2
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
Michael Haggerty· Mar 5, 2014, 17:26 UTC · re: Michael Haggerty · lore

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

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(-)
Show changes to cache-tree.c +3 −2
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

← back to recent threads