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

> Signed-off-by: Michael Haggerty <mhag...@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 == '/')
                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 == '/')
                if (!*slash)
                        return it;

David Kastrup
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to