Subject: Re: [PATCH v2 02/21] t/helper/ref-store: initialize oid in resolve-ref

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Thu, 20 May 2021 17:06:59 +0200

On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <[email-masked]>
>
> This will print $ZERO_OID when asking for a non-existent ref from the
> test-helper.
>
> Since resolve-ref provides direct access to refs_resolve_ref_unsafe(), it
> provides a reliable mechanism for accessing REFNAME, while avoiding the implicit
> resolution to refs/heads/REFNAME.
>
> Signed-off-by: Han-Wen Nienhuys <[email-masked]>
> ---
>  t/helper/test-ref-store.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
> index bba5f841c6ab..01d8f3285dc8 100644
> --- a/t/helper/test-ref-store.c
> +++ b/t/helper/test-ref-store.c
> @@ -118,7 +118,7 @@ static int cmd_for_each_ref(struct ref_store *refs, const char **argv)
>  
>  static int cmd_resolve_ref(struct ref_store *refs, const char **argv)
>  {
> -	struct object_id oid;
> +	struct object_id oid = { 0 };
>  	const char *refname = notnull(*argv++, "refname");
>  	int resolve_flags = arg_flags(*argv++, "resolve-flags");
>  	int flags;

This feels a bit magical, later we have this:

        printf("%s %s 0x%x\n", oid_to_hex(&oid), ref ? ref : "(null)", flags);

Isn't ref always going to be NULL in that case too? Wouldn't it make
more sense to not zero this out and instead do:

    if (ref)
        /* current code, mostly */
    else
        use zero_oid()

That seems more straightforward to me than this implicit proxy for
zero_oid(). Also, isn't the point of zero_oid() to not make this
particular assumption, i.e. the recent discussion (haven't followed
where it went) of the "oid" having some sort of "hash type" member,
which surely would do the wrong thing here under either SHA-1 or
SHA-256, or maybe I mis(understood|remember) that discussion...