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@xxxxxxxxx>
---
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
