From: Karthik Nayak Date: Mon, 23 Feb 2026 09:09:15 GMT Subject: Re: [PATCH 10/17] refs: improve verification for-each-ref options Message-ID: In-Reply-To: <20260220-pks-refs-for-each-unification-v1-10-17170bd99de1@pks.im> Patrick Steinhardt writes: > Improve verification of the passed-in for-each-ref options: > > - Require that the `refs` store must be given. It's arguably very > surprising that we simply return successfully in case the ref store > is a `NULL` pointer. > > - When expected to trim ref prefixes we will `BUG()` in case the > refname would become empty or in case we're expected to trim a > longer prefix than the refname is long. As such, this case is only > guaranteed to _not_ `BUG()` in case the caller also specified a > prefix. And furthermore, that prefix must end in a trailing slash, > as otherwise it may produce an exact match that could lead us to > trim to the empty string. > > An audit shows that there are no callsites that rely on either of these > behaviours, so this should not result in a functional change. > > Signed-off-by: Patrick Steinhardt > --- > refs.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/refs.c b/refs.c > index 20d34faeb5..3b676432b4 100644 > --- a/refs.c > +++ b/refs.c > @@ -1855,7 +1855,18 @@ int refs_for_each_ref_ext(struct ref_store *refs, > int ret; > > if (!refs) > - return 0; > + BUG("no refs passed"); > + Nit: s/refs/ref store/, mostly from a readability point, but since this is a BUG(), I think its okay to leave as is. > + if (opts->trim_prefix) { > + size_t prefix_len; > + > + if (!opts->prefix) > + BUG("trimming only allowed with a prefix"); > + > + prefix_len = strlen(opts->prefix); > + if (prefix_len == opts->trim_prefix && opts->prefix[prefix_len - 1] != '/') > + BUG("ref pattern must end in a trailing slash when trimming"); > + } > > if (opts->pattern) { > if (!opts->prefix && !starts_with(opts->pattern, "refs/")) > > -- > 2.53.0.414.gf7e9f6c205.dirty