Re: [PATCH v3 1/3] refs: keep track of unresolved reference value in iterators
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 7, 2024, 21:40 UTC
- Message-ID
- <xmqqbk24nhad.fsf@gitster.g>
- In-Reply-To
- <fc3defd9c47e32bb23ba0fcb5c885274f3706b23.1723059769.git.gitgitgadget@gmail.com>
"John Cai via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 10 quoted lines
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index aa52d9be7c7..5ed69c23f74 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -245,9 +245,11 @@ static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
> {
> struct object_id oid;
> int flag;
> -
> - if (!refs_resolve_ref_unsafe(&refs->base, refname, RESOLVE_REF_READING,Here, we had a nice blank line that separated the decls and the first statement.
Show 6 quoted lines
> - &oid, &flag)) {
> + const char *referent = refs_resolve_ref_unsafe(&refs->base,
> + refname,
> + RESOLVE_REF_READING,
> + &oid, &flag);
> + if (!referent) {We lost it here, though.
Show 11 quoted lines
> oidclr(&oid, the_repository->hash_algo);
> flag |= REF_ISBROKEN;
> } else if (is_null_oid(&oid)) {
> @@ -268,7 +270,11 @@ static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
> oidclr(&oid, the_repository->hash_algo);
> flag |= REF_BAD_NAME | REF_ISBROKEN;
> }
> - add_entry_to_dir(dir, create_ref_entry(refname, &oid, flag));
> +
> + if (!(flag & REF_ISSYMREF))
> + referent = NULL;OK, this is new in this round. The idea is that everybody else can rely on the invariant that the referent being NULL is equivalent to REF_ISSYMREF bit in flag word being off from here on.
Show 14 quoted lines
> + add_entry_to_dir(dir, create_ref_entry(refname, referent, &oid, flag)); > } > > /* > @@ -886,6 +892,11 @@ static int files_ref_iterator_advance(struct ref_iterator *ref_iterator) > iter->base.refname = iter->iter0->refname; > iter->base.oid = iter->iter0->oid; > iter->base.flags = iter->iter0->flags; > + if (iter->iter0->flags & REF_ISSYMREF) > + iter->base.referent = iter->iter0->referent; > + else > + iter->base.referent = NULL; > return ITER_OK; > }
Hmph, why not an unconditional
iter->base.referent = iter->iter0->referent;
instead? This code is making sure (iter->base.flags & REF_ISSYMREF) is directly linked to non-NULL-ness or iter->base.referent, and we want to make everybody take it as invariant. Shouldn't this code also rely on the same invariant? If iter-iter0->referent is NULL, iter->iter0->flag has REF_ISSYMREF bit off, and vice versa.