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

From: David Kastrup <dak@gnu.org>
Date: Mar 4, 2014, 09:40 UTC
Message-ID: <87ppm2i9ot.fsf@fencepost.gnu.org>
In-Reply-To: <1393921868-4382-1-git-send-email-mhagger@alum.mit.edu>

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

Show 10 quoted lines

> 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