Re: [PATCH v1 1/2] refs: extract function to normalize partial refs

[Kevin Daudt <me@ikke.info>]

On Sat, Nov 04, 2017 at 11:27:39AM +0900, Junio C Hamano wrote:
> I however notice that addition of /* to the tail is trying to be
> careful by using strbuf_complete('/'), but prefixing with "refs/"
> does not and we would end up with a double-slash if pattern begins
> with a slash.  The contract between the caller of this function (or
> its original, which is for_each_glob_ref_in()) and the callee is
> that prefix must not begin with '/', so it may be OK, but we might
> want to add "if (*pattern == '/') BUG(...)" at the beginning.
>
> I dunno.  In any case, that is totally outside the scope of this two
> patch series.

I do think it's a good idea to make future readers of the code aware of
this contract, and adding a BUG assert does that quite well. Here is a
patch that implements it.

This applies of course on top of this patch series.

-- >8 --
Subject: [PATCH] normalize_glob_ref: assert implicit contract of prefix

normalize_glob_ref has an implicit contract of expecting 'prefix' to not
start with a '/', otherwise the pattern would end up with a
double-slash.

Mark it as a BUG when the prefix argument of normalize_glob_ref starts
with a '/' so that future callers will be aware of this contract.

Signed-off-by: Kevin Daudt <me@ikke.info>
---
 refs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/refs.c b/refs.c
index e9ae659ae..6747981d1 100644
--- a/refs.c
+++ b/refs.c
@@ -372,6 +372,8 @@ int head_ref_namespaced(each_ref_fn fn, void *cb_data)
 void normalize_glob_ref(struct strbuf *normalized_pattern, const char *prefix,
 		const char *pattern, int flags)
 {
+	if (prefix && *prefix == '/') BUG("prefix cannot not start with '/'");
+
 	if (!prefix && !starts_with(pattern, "refs/"))
 		strbuf_addstr(normalized_pattern, "refs/");
 	else if (prefix)
-- 
2.15.0.rc2.57.g2f899857a9