Re: [PATCH v3 1/4] refs: introduce `is_pseudoref()` and `is_headref()`
- From
Jeff King <peff@peff.net>
- Date
- Feb 7, 2024, 01:48 UTC
- Message-ID
- <20240207014819.GA1399428@coredump.intra.peff.net>
- In-Reply-To
- <20240129113527.607022-2-karthik.188@gmail.com>
On Mon, Jan 29, 2024 at 12:35:24PM +0100, Karthik Nayak wrote:
Show 7 quoted lines
> +int is_pseudoref(struct ref_store *refs, const char *refname)
> [...]
> + if (ends_with(refname, "_HEAD")) {
> + read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
> + &oid, NULL);
> + return !is_null_oid(&oid);
> + }I was going to prepare a patch on top, but since it looks like this was reverted out of 'next' to be revamped, I thought I'd mention it now: -Wunused-parameter notices that we never use the "refs" parameter to the function. And indeed it looks like a (possible) bug, since read_ref_full() is going to use the_repository to find the ref store.
I think you'd want something like this squashed in (note that it also fixes a slight indent problem in the first block):
diff --git a/refs.c b/refs.c index 3190df8d81..0d65c31117 100644 --- a/refs.c +++ b/refs.c @@ -875,15 +875,17 @@ int is_pseudoref(struct ref_store *refs, const char *refname) return 0; if (ends_with(refname, "_HEAD")) { - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); - return !is_null_oid(&oid); + refs_resolve_ref_unsafe(refs, refname, + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); + return !is_null_oid(&oid); } for (i = 0; i < ARRAY_SIZE(irregular_pseudorefs); i++) if (!strcmp(refname, irregular_pseudorefs[i])) { - read_ref_full(refname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, - &oid, NULL); + refs_resolve_ref_unsafe(refs, refname, + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, + &oid, NULL); return !is_null_oid(&oid); } -Peff