# [PATCH] cache_tree_find(): remove redundant check in while condition

4 messages from 2014-03-03 to 2014-03-03. Participants: Michael Haggerty, David Kastrup.
Thread: https://gitlist.dev/t/36040

## Michael Haggerty, 2014-03-03 16:08

Subject: [PATCH] cache_tree_find(): remove redundant check in while condition
Message-ID: <1393862885-23271-1-git-send-email-mhagger@alum.mit.edu>
URL: https://gitlist.dev/e/1393862885-23271-1-git-send-email-mhagger%40alum.mit.edu

```
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
A trivial little cleanup.

 cache-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

```

## David Kastrup, 2014-03-03 16:32

Subject: Re: [PATCH] cache_tree_find(): remove redundant check in while condition
Message-ID: <87a9d7kztm.fsf@fencepost.gnu.org>
URL: https://gitlist.dev/e/87a9d7kztm.fsf%40fencepost.gnu.org
In-Reply-To: <1393862885-23271-1-git-send-email-mhagger@alum.mit.edu>

```
Michael Haggerty <mhagger@alum.mit.edu> writes:

> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> A trivial little cleanup.
>
>  cache-tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cache-tree.c b/cache-tree.c
> index 0bbec43..7f63c23 100644
> --- a/cache-tree.c
> +++ b/cache-tree.c
> @@ -565,7 +565,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree *it, const char *pat
>  			return NULL;
>  		it = sub->cache_tree;
>  		if (slash)
> -			while (*slash && *slash == '/')
> +			while (*slash == '/')
>  				slash++;
>  		if (!slash || !*slash)
>  			return it; /* prefix ended with slashes */

That seems dragging around a NULL slash a lot.  How about not checking
for it multiple times?

		if (!slash)
                	return it;
		while (*slash == '/')
			slash++;
		if (!*slash)
			return it; /* prefix ended with slashes */

As a bonus, the comment appears to be actually correct.  Attempting to
verify its correctness by seeing whether a non-NULL slash is guaranteed
to really end with slashes, we find the following code above for
defining slash:

                slash = strchr(path, '/');
                if (!slash)
                        slash = path + strlen(path);

So it is literally impossible for slash to ever be NULL and all the
checking is nonsensical.  In addition, "prefix ended with slashes" does
not seem overly convincing when this code path is reached whether or not
there is a slash at all (I am not sure about it: it depends on the
preceding find_subtree to some degree).

So perhaps all of that should just be

		while (*slash == '/')
			slash++;
		if (!*slash)
			return it;

-- 
David Kastrup

```

## Michael Haggerty, 2014-03-03 17:07

Subject: Re: [PATCH] cache_tree_find(): remove redundant check in while condition
Message-ID: <5314B6E4.7030607@alum.mit.edu>
URL: https://gitlist.dev/e/5314B6E4.7030607%40alum.mit.edu
In-Reply-To: <87a9d7kztm.fsf@fencepost.gnu.org>

```
On 03/03/2014 05:32 PM, David Kastrup wrote:
> [...]
> So perhaps all of that should just be
> 
> 		while (*slash == '/')
> 			slash++;
> 		if (!*slash)
> 			return it;
> 

I just fixed a little thing that caught my eye.  You OWNED it.  You are
absolutely right.

Will you prepare a patch or would you like me to do it?

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

```

## David Kastrup, 2014-03-03 17:25

Subject: Re: [PATCH] cache_tree_find(): remove redundant check in while condition
Message-ID: <8761nvkxcv.fsf@fencepost.gnu.org>
URL: https://gitlist.dev/e/8761nvkxcv.fsf%40fencepost.gnu.org
In-Reply-To: <5314B6E4.7030607@alum.mit.edu>

```
Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 03/03/2014 05:32 PM, David Kastrup wrote:
>> [...]
>> So perhaps all of that should just be
>> 
>> 		while (*slash == '/')
>> 			slash++;
>> 		if (!*slash)
>> 			return it;
>> 
>
> I just fixed a little thing that caught my eye.  You OWNED it.  You are
> absolutely right.
>
> Will you prepare a patch or would you like me to do it?

Feel free.  I really should be finishing some other patches instead...

-- 
David Kastrup

```