public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: David Kastrup <dak@gnu•org>
To: Michael Haggerty <mhagger@alum•mit.edu>
Cc: Junio C Hamano <gitster@pobox•com>, git@vger•kernel.org
Subject: Re: [PATCH] cache_tree_find(): remove redundant check in while condition
Date: Mon, 03 Mar 2014 17:32:37 +0100	[thread overview]
Message-ID: <87a9d7kztm.fsf@fencepost.gnu.org> (raw)
In-Reply-To: <1393862885-23271-1-git-send-email-mhagger@alum.mit.edu> (Michael Haggerty's message of "Mon, 3 Mar 2014 17:08:05 +0100")

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

next prev parent reply	other threads:[~2014-03-03 16:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-03 16:08 [PATCH] cache_tree_find(): remove redundant check in while condition Michael Haggerty
2014-03-03 16:32 ` David Kastrup [this message]
2014-03-03 17:07   ` Michael Haggerty
2014-03-03 17:25     ` David Kastrup

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a9d7kztm.fsf@fencepost.gnu.org \
    --to=dak@gnu$(echo .)org \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=mhagger@alum$(echo .)mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox