threads / patch / 46439

v2refs: use skip_prefix() in ref_is_hidden()

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

## tl;dr

2 messages between Jul 22, 2017 and Jul 24, 2017. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Christian Couder· Jul 22, 2017, 04:39 UTC · lore

This is shorter, makes the logic a bit easier to follow, and is perhaps a bit faster too.

The logic is to 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).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---

The change with the previous version was suggested by Junio in

https://public-inbox.org/git/xmqqk231hgpz.fsf@gitster.mtv.corp.google.com/

 refs.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Show changes to refs.c +4 −5

diff --git a/refs.c b/refs.c
index ba22f4acef..ea2b9f84f8 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))
-			continue;
-		len = strlen(match);
-		if (!subject[len] || subject[len] == '/')
+		if (subject &&
+		    skip_prefix(subject, match, &p) &&
+		    (!*p || *p == '/'))
 			return !neg;
 	}
 	return 0;

-- 
2.14.0.rc0.26.g981adb928e.dirty

Jeff King· Jul 24, 2017, 16:09 UTC · re: Christian Couder · lore

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

On Sat, Jul 22, 2017 at 06:39:12AM +0200, Christian Couder wrote:

Show 12 quoted lines

> @@ -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))
> -			continue;
> -		len = strlen(match);
> -		if (!subject[len] || subject[len] == '/')
> +		if (subject &&
> +		    skip_prefix(subject, match, &p) &&
> +		    (!*p || *p == '/'))
>  			return !neg;

This looks good to me.

I seem to recall running across a similar pattern elsewhere, where a caller wanted either an exact match, or a match ending with a particular character. We could add one more helper like:

  if (subject && skip_prefix_with_sep(subject, match, '/', &p))
	return !neg;

But I don't offhand recall where that other place was, and it's not like it's saving a huge amount of complexity in the caller. So I'll file it away in the back of my mind and see if it comes up a third time. ;)

-Peff