Re: [PATCH 2/4] refs: keep track of unresolved reference value in iterators

From: Jeff King <peff@peff.net>
Date: Jun 11, 2024, 09:01 UTC
Message-ID: <20240611090156.GK3248245@coredump.intra.peff.net>
In-Reply-To: <7202ada0542b6f014647785945094a13c9d885c7.1717694802.git.gitgitgadget@gmail.com>

On Thu, Jun 06, 2024 at 05:26:38PM +0000, John Cai via GitGitGadget wrote:

Show 15 quoted lines

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index bf2ffe062ea..a963d796a29 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -243,8 +243,9 @@ static void loose_fill_ref_dir_regular_file(struct files_ref_store *refs,
>  {
>  	struct object_id oid;
>  	int flag;
> +	const char* referent = NULL;
>  
> -	if (!refs_resolve_ref_unsafe(&refs->base, refname, NULL, RESOLVE_REF_READING,
> +	if (!refs_resolve_ref_unsafe(&refs->base, refname, referent, RESOLVE_REF_READING,
>  				     &oid, &flag)) {
>  		oidclr(&oid);
>  		flag |= REF_ISBROKEN;

Here we pass in NULL, so the code in refs_resolve_ref_unsafe() won't do anything. And our copy of "referent" here will remain NULL, so the rest of this patch also does nothing. Again, I think that the function should take a "char **", and you'd pass in &referent here?

Though if we are OK with surfacing just the final value in a multi-element chain, then you could just use the existing return value, like:

  referent = refs_resolve_ref_unsafe(&refs->base, refname,
				     RESOLVE_REF_REAEDING, &oid, &flags);
  if (!referent) {
          oidclr(&oid);
          flag |= REF_ISBROKEN;
  }

and then later pass "referent" to create_ref_entry() if flags contains REF_ISSYMREF (or since we pass it the flags, it could do that check itself).

-Peff