Thread (129 messages) 129 messages, 6 authors, 2021-06-02

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

From: Han-Wen Nienhuys <hidden>
Date: 2021-05-31 16:18:23

On Thu, May 20, 2021 at 5:09 PM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Tue, Apr 27 2021, Han-Wen Nienhuys via GitGitGadget wrote:
quoted
From: Han-Wen Nienhuys <redacted>

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 <redacted>
---
 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()
for programmatic access, the if will actually make it harder to parse
out what is happening, so I think this solution is better. Note that
the function already handles ref == null, so changing the printf
format in this way can only serve to break other tests.
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
used null_oid() instead (zero_oid() doesn't exist in C).

-- 
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help