Re: [PATCH] refs: use skip_prefix() in ref_is_hidden()

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 21, 2017, 16:55 UTC
Message-ID: <xmqqk231hgpz.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170721095757.28373-1-chriscool@tuxfamily.org>

Christian Couder <christian.couder@gmail.com> writes:

Show 34 quoted lines

> This saves one line, makes the code a bit easier to understand
> and perhaps a bit faster too.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  refs.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index ba22f4acef..15cb36d426 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1160,7 +1160,7 @@ int ref_is_hidden(const char *refname, const char *refname_full)
>  		const char *match = hide_refs->items[i].string;
>  		const char *subject;
>  		int neg = 0;
> -		int len;
> +		const char *p;
>  
>  		if (*match == '!') {
>  			neg = 1;
> @@ -1175,10 +1175,9 @@ int ref_is_hidden(const char *refname, const char *refname_full)
>  		}
>  
>  		/* refname can be NULL when namespaces are used. */
> -		if (!subject || !starts_with(subject, match))
> +		if (!subject || !skip_prefix(subject, match, &p))
>  			continue;
> -		len = strlen(match);
> -		if (!subject[len] || subject[len] == '/')
> +		if (!*p || *p == '/')
>  			return !neg;
>  	}
>  	return 0;

If we were to rewrite this using skip_prefix(), I would imagine we would rather write it this way:

	if (subject &&
	    skip_prefix(subject, match, &p) &&
	    (*p || *p != '/'))
		return !neg;

because it would be shorter and make the logic easier to follow:

    We make the final decision only when "subject" is there, its
    early part matches "match", and the match is at the slash
    boundary (or the whole thing).

The original wanted to say the same thing, but it had to split that logic into two only because it needed an assignment to "len" in the middle, and because doing an assignment inside a conditional part of if() statement is frowned upon.