refs: drop unused params from the reflog iterator callback

The ref and reflog iterators share much of the same underlying code to
iterate over the corresponding entries. This results in some weird code
because the reflog iterator also exposes an object ID as well as a flag
to the callback function. Neither of these fields do refer to the reflog
though -- they refer to the corresponding ref with the same name. This
is quite misleading. In practice at least the object ID cannot really be
implemented in any other way as a reflog does not have a specific object
ID in the first place. This is further stressed by the fact that none of
the callbacks except for our test helper make use of these fields.

Split up the infrastucture so that ref and reflog iterators use separate
callback signatures. This allows us to drop the nonsensical fields from
the reflog iterator.

Note that internally, the backends still use the same shared infra to
iterate over both types. As the backends should never end up being
called directly anyway, this is not much of a problem and thus kept
as-is for simplicity's sake.

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

diff --git a/builtin/fsck.c b/builtin/fsck.c
index a7cf94f67edf3aaace9fdbbca29778d6857d28af..f892487c9b975dd5079b86143e635c6539770981 100644 (file)
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -509,9 +509,7 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
        return 0;
 }
 
-static int fsck_handle_reflog(const char *logname,
-                             const struct object_id *oid UNUSED,
-                             int flag UNUSED, void *cb_data)
+static int fsck_handle_reflog(const char *logname, void *cb_data)
 {
        struct strbuf refname = STRBUF_INIT;
 

diff --git a/builtin/reflog.c b/builtin/reflog.c
index a5a4099f61a5f825d6e622572c9ee85ae563de7b..3a0c4d432232bdc8680fa78d22baf35ac187c5b1 100644 (file)
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -60,8 +60,7 @@ struct worktree_reflogs {
        struct string_list reflogs;
 };
 
-static int collect_reflog(const char *ref, const struct object_id *oid UNUSED,
-                         int flags UNUSED, void *cb_data)
+static int collect_reflog(const char *ref, void *cb_data)
 {
        struct worktree_reflogs *cb = cb_data;
        struct worktree *worktree = cb->worktree;

diff --git a/refs.c b/refs.c
index dc25606a82648c189b6d6376189425c5fec4a68e..f9261267f05efcee5be1f312a1a82b034bff61b6 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2512,18 +2512,33 @@ cleanup:
        return ret;
 }
 
-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)
+struct do_for_each_reflog_help {
+       each_reflog_fn *fn;
+       void *cb_data;
+};
+
+static int do_for_each_reflog_helper(struct repository *r UNUSED,
+                                    const char *refname,
+                                    const struct object_id *oid UNUSED,
+                                    int flags,
+                                    void *cb_data)
+{
+       struct do_for_each_reflog_help *hp = cb_data;
+       return hp->fn(refname, hp->cb_data);
+}
+
+int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data)
 {
        struct ref_iterator *iter;
-       struct do_for_each_ref_help hp = { fn, cb_data };
+       struct do_for_each_reflog_help hp = { fn, cb_data };
 
        iter = refs->be->reflog_iterator_begin(refs);
 
        return do_for_each_repo_ref_iterator(the_repository, iter,
-                                            do_for_each_ref_helper, &hp);
+                                            do_for_each_reflog_helper, &hp);
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int for_each_reflog(each_reflog_fn fn, void *cb_data)
 {
        return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);
 }

diff --git a/refs.h b/refs.h
index 303c5fac4d08b9798fafb6d4c667e9a94bf91e72..895579aeb7a9da56995550f7631caf55371c7951 100644 (file)
--- a/refs.h
+++ b/refs.h
@@ -534,12 +534,19 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat
 /* youngest entry first */
 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);
 
+/*
+ * The signature for the callback function for the {refs_,}for_each_reflog()
+ * functions below. The memory pointed to by the refname argument is only
+ * guaranteed to be valid for the duration of a single callback invocation.
+ */
+typedef int each_reflog_fn(const char *refname, void *cb_data);
+
 /*
  * Calls the specified function for each reflog file until it returns nonzero,
  * and returns the value. Reflog file order is unspecified.
  */
-int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);
-int for_each_reflog(each_ref_fn fn, void *cb_data);
+int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data);
+int for_each_reflog(each_reflog_fn fn, void *cb_data);
 
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 05bb0c875cf16b96b34a6318aaa17df5b8dd266c..c7aff6b331bd4150f9ebb32acd2a876a1cc485a3 100644 (file)
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2115,10 +2115,8 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
 
 struct files_reflog_iterator {
        struct ref_iterator base;
-
        struct ref_store *ref_store;
        struct dir_iterator *dir_iterator;
-       struct object_id oid;
 };
 
 static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
@@ -2129,8 +2127,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
        int ok;
 
        while ((ok = dir_iterator_advance(diter)) == ITER_OK) {
-               int flags;
-
                if (!S_ISREG(diter->st.st_mode))
                        continue;
                if (diter->basename[0] == '.')
@@ -2140,14 +2136,12 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 
                if (!refs_resolve_ref_unsafe(iter->ref_store,
                                             diter->relative_path, 0,
-                                            &iter->oid, &flags)) {
+                                            NULL, NULL)) {
                        error("bad ref for %s", diter->path.buf);
                        continue;
                }
 
                iter->base.refname = diter->relative_path;
-               iter->base.oid = &iter->oid;
-               iter->base.flags = flags;
                return ITER_OK;
        }
 

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 39e9a9d4e250436a44ce728c5c6c98dd91ce6932..4998b676c231ecd9cfa5a7cb1385643013e08e57 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1594,7 +1594,6 @@ struct reftable_reflog_iterator {
        struct reftable_ref_store *refs;
        struct reftable_iterator iter;
        struct reftable_log_record log;
-       struct object_id oid;
        char *last_name;
        int err;
 };
@@ -1605,8 +1604,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
                (struct reftable_reflog_iterator *)ref_iterator;
 
        while (!iter->err) {
-               int flags;
-
                iter->err = reftable_iterator_next_log(&iter->iter, &iter->log);
                if (iter->err)
                        break;
@@ -1620,7 +1617,7 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
                        continue;
 
                if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname,
-                                            0, &iter->oid, &flags)) {
+                                            0, NULL, NULL)) {
                        error(_("bad ref for %s"), iter->log.refname);
                        continue;
                }
@@ -1628,8 +1625,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
                free(iter->last_name);
                iter->last_name = xstrdup(iter->log.refname);
                iter->base.refname = iter->log.refname;
-               iter->base.oid = &iter->oid;
-               iter->base.flags = flags;
 
                break;
        }
@@ -1682,7 +1677,6 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
        iter = xcalloc(1, sizeof(*iter));
        base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable);
        iter->refs = refs;
-       iter->base.oid = &iter->oid;
 
        ret = refs->err;
        if (ret)

diff --git a/revision.c b/revision.c
index 2424c9bd674e534909df89e25c21b5eb119fda05..ac45c6d8f29639dbb5cdd759f5339726a7d4dfe5 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1686,9 +1686,7 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
        return 0;
 }
 
-static int handle_one_reflog(const char *refname_in_wt,
-                            const struct object_id *oid UNUSED,
-                            int flag UNUSED, void *cb_data)
+static int handle_one_reflog(const char *refname_in_wt, void *cb_data)
 {
        struct all_refs_cb *cb = cb_data;
        struct strbuf refname = STRBUF_INIT;

diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 702ec1f128ad42443bc093614dee971c7106af77..7a0f6cac53d2433ade595ce95ab2d30e214b2ede 100644 (file)
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -221,15 +221,21 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)
        return ret;
 }
 
+static int each_reflog(const char *refname, void *cb_data UNUSED)
+{
+       printf("%s\n", refname);
+       return 0;
+}
+
 static int cmd_for_each_reflog(struct ref_store *refs,
                               const char **argv UNUSED)
 {
-       return refs_for_each_reflog(refs, each_ref, NULL);
+       return refs_for_each_reflog(refs, each_reflog, NULL);
 }
 
-static int each_reflog(struct object_id *old_oid, struct object_id *new_oid,
-                      const char *committer, timestamp_t timestamp,
-                      int tz, const char *msg, void *cb_data UNUSED)
+static int each_reflog_ent(struct object_id *old_oid, struct object_id *new_oid,
+                          const char *committer, timestamp_t timestamp,
+                          int tz, const char *msg, void *cb_data UNUSED)
 {
        printf("%s %s %s %" PRItime " %+05d%s%s", oid_to_hex(old_oid),
               oid_to_hex(new_oid), committer, timestamp, tz,
@@ -241,14 +247,14 @@ static int cmd_for_each_reflog_ent(struct ref_store *refs, const char **argv)
 {
        const char *refname = notnull(*argv++, "refname");
 
-       return refs_for_each_reflog_ent(refs, refname, each_reflog, refs);
+       return refs_for_each_reflog_ent(refs, refname, each_reflog_ent, refs);
 }
 
 static int cmd_for_each_reflog_ent_reverse(struct ref_store *refs, const char **argv)
 {
        const char *refname = notnull(*argv++, "refname");
 
-       return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog, refs);
+       return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog_ent, refs);
 }
 
 static int cmd_reflog_exists(struct ref_store *refs, const char **argv)

diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index 4f860285cce8de99f494b9c10f3e08b8c480ba4b..56a3196b836aac08ac9d53dc9e655f536cb7fac8 100755 (executable)
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -287,23 +287,23 @@ test_expect_success 'for_each_reflog()' '
        mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
        echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
 
-       $RWT for-each-reflog | cut -d" " -f 2- >actual &&
+       $RWT for-each-reflog >actual &&
        cat >expected <<-\EOF &&
-       HEAD 0x1
-       PSEUDO-WT 0x0
-       refs/bisect/wt-random 0x0
-       refs/heads/main 0x0
-       refs/heads/wt-main 0x0
+       HEAD
+       PSEUDO-WT
+       refs/bisect/wt-random
+       refs/heads/main
+       refs/heads/wt-main
        EOF
        test_cmp expected actual &&
 
-       $RMAIN for-each-reflog | cut -d" " -f 2- >actual &&
+       $RMAIN for-each-reflog >actual &&
        cat >expected <<-\EOF &&
-       HEAD 0x1
-       PSEUDO-MAIN 0x0
-       refs/bisect/random 0x0
-       refs/heads/main 0x0
-       refs/heads/wt-main 0x0
+       HEAD
+       PSEUDO-MAIN
+       refs/bisect/random
+       refs/heads/main
+       refs/heads/wt-main
        EOF
        test_cmp expected actual
 '

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index cfb583f54499e60108ed98d3b54a5925356b522b..3eee758bceefae2fb8cc27ea8387645dbbe73133 100755 (executable)
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -74,11 +74,11 @@ test_expect_success 'verify_ref(new-main)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-       $RUN for-each-reflog | cut -d" " -f 2- >actual &&
+       $RUN for-each-reflog >actual &&
        cat >expected <<-\EOF &&
-       HEAD 0x1
-       refs/heads/main 0x0
-       refs/heads/new-main 0x0
+       HEAD
+       refs/heads/main
+       refs/heads/new-main
        EOF
        test_cmp expected actual
 '

diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index 40332e23cc43b338b0d8d8c729f230603649233b..c01f0f14a1266f93f9066c738693d7c8a9a526dc 100755 (executable)
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -63,11 +63,11 @@ test_expect_success 'verify_ref(new-main)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-       $RUN for-each-reflog | cut -d" " -f 2- >actual &&
+       $RUN for-each-reflog >actual &&
        cat >expected <<-\EOF &&
-       HEAD 0x1
-       refs/heads/main 0x0
-       refs/heads/new-main 0x0
+       HEAD
+       refs/heads/main
+       refs/heads/new-main
        EOF
        test_cmp expected actual
 '