refs: fully reset `struct ref_iterator::ref` on iteration

With the introduction of the `struct ref_iterator::ref` field it now is
a whole lot easier to introduce new fields that become accessible to the
caller without having to adapt every single callsite. But there's a
downside: when a new field is introduced we always have to adapt all
backends to set that field.

This isn't something we can avoid in the general case: when the new
field is expected to be populated by all backends we of course cannot
avoid doing so. But new fields may be entirely optional, in which case
we'd still have such churn. And furthermore, it is very easy right now
to leak state from a previous iteration into the next iteration.

Address this issue by ensuring that the reference backends all fully
reset the field on every single iteration. This ensures that no state
from previous iterations can leak into the next one. And it ensures that
any newly introduced fields will be zeroed out by default.

Note that we don't have to explicitly adapt the "files" backend, as it
uses the `cache_ref_iterator` internally. Furthermore, other "wrapping"
iterators like for example the `prefix_ref_iterator` copy around the
whole reference, so these don't need to be adapted either.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 7987acdc96a14b4bb5356a5e740b5848d9e2487a..711e07f8326c6bdb89ff40fd20816c2508570c37 100644 (file)
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -882,6 +882,7 @@ static int next_record(struct packed_ref_iterator *iter)
 {
        const char *p, *eol;
 
+       memset(&iter->base.ref, 0, sizeof(iter->base.ref));
        strbuf_reset(&iter->refname_buf);
 
        /*
@@ -916,6 +917,7 @@ static int next_record(struct packed_ref_iterator *iter)
            !isspace(*p++))
                die_invalid_line(iter->snapshot->refs->path,
                                 iter->pos, iter->eof - iter->pos);
+       iter->base.ref.oid = &iter->oid;
 
        eol = memchr(p, '\n', iter->eof - p);
        if (!eol)
@@ -1194,7 +1196,6 @@ static struct ref_iterator *packed_ref_iterator_begin(
        iter->snapshot = snapshot;
        acquire_snapshot(snapshot);
        strbuf_init(&iter->refname_buf, 0);
-       iter->base.ref.oid = &iter->oid;
        iter->repo = ref_store->repo;
        iter->flags = flags;
 

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index f1abc39624166e50cb8012b449142fe780393b29..e427848879d61b2f094a494167e4bfd03fb70592 100644 (file)
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -425,6 +425,7 @@ static int cache_ref_iterator_advance(struct ref_iterator *ref_iterator)
                        level->prefix_state = entry_prefix_state;
                        level->index = -1;
                } else {
+                       memset(&iter->base.ref, 0, sizeof(iter->base.ref));
                        iter->base.ref.name = entry->name;
                        iter->base.ref.target = entry->u.value.referent;
                        iter->base.ref.oid = &entry->u.value.oid;

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 0e47986cb5b69955e825c4eb1338b97e129f15e5..728886eafd33bd9aa8138f47e5803ff360a054ba 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -704,6 +704,7 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
                                            &iter->oid, flags))
                                continue;
 
+               memset(&iter->base.ref, 0, sizeof(iter->base.ref));
                iter->base.ref.name = iter->ref.refname;
                iter->base.ref.target = referent;
                iter->base.ref.oid = &iter->oid;