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

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

[Christian Couder]

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;
-- 
2.14.0.rc0.26.g981adb928e.dirty

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

[Junio C Hamano]

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

> 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.