Re: [PATCH v2] cache_tree_find(): remove redundant checks

[David Kastrup <dak@gnu.org>]

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

> The beginning of the loop ensures that slash can never be NULL.  So
> don't keep checking whether it is NULL later in the loop.
>
> Furthermore, 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.

Hm.  Another suggestion.  You have

		const char *slash = strchr(path, '/');
 		if (!slash)
 			slash = path + strlen(path);
[...]
		sub = find_subtree(it, path, slash - path, 0);
[...]
		path = slash;
		while (*path == '/')
			path++;
	}

At the price of introducing another variable, this could be

		const char *slash = strchr(path, '/');
 		size_t len = slash ? slash - path : strlen(path);
[...]
		sub = find_subtree(it, path, len, 0);
[...]
                if (!slash)
			break;
		for (path = slash; *path == '/';)
			path++;
	}

This introduces another variable and another condition.  The advantage
is that "slash" indeed points at a slash or is NULL, so the variable
names correspond better to what happens.  Alternatively, it might make
sense to rename "slash" into "end" or "endpart" or whatever.  Since
I can't think of a pretty name, I lean towards preferring the latter
version as it reads nicer.  I prefer code to read like children's books
rather than mystery novels.

-- 
David Kastrup