# [PATCH v3 1/6] cache_tree_find(): remove redundant checks 7 messages from 2014-03-05 to 2014-03-05. Participants: Michael Haggerty. Thread: https://gitlist.dev/t/36066 ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 0/6] cache_tree_find() workover Message-ID: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-1-git-send-email-mhagger%40alum.mit.edu ``` 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, 2014-03-05 17:26 Subject: [PATCH v3 1/6] cache_tree_find(): remove redundant checks Message-ID: <1394040390-7954-2-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-2-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` 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 Signed-off-by: Michael Haggerty --- 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 ``` ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 2/6] cache_tree_find(): initialize slash using strchrnul() Message-ID: <1394040390-7954-3-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-3-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` Suggested-by: Junio Hamano Signed-off-by: Michael Haggerty --- 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 ``` ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 3/6] cache_tree_find(): fix comment formatting Message-ID: <1394040390-7954-4-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-4-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` Signed-off-by: Michael Haggerty --- 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 ``` ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 4/6] cache_tree_find(): remove redundant check Message-ID: <1394040390-7954-5-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-5-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` If *slash == '/', then it is necessarily non-NUL. Signed-off-by: Michael Haggerty --- 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 ``` ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 5/6] cache_tree_find(): remove early return Message-ID: <1394040390-7954-6-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-6-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` 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 --- 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 ``` ## Michael Haggerty, 2014-03-05 17:26 Subject: [PATCH v3 6/6] cache_tree_find(): use path variable when passing over slashes Message-ID: <1394040390-7954-7-git-send-email-mhagger@alum.mit.edu> URL: https://gitlist.dev/e/1394040390-7954-7-git-send-email-mhagger%40alum.mit.edu In-Reply-To: <1394040390-7954-1-git-send-email-mhagger@alum.mit.edu> ``` 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 --- 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 ```