oidtree: extend iteration to allow for arbitrary return codes

The interface `cb_each()` iterates through a crit-bit tree and calls a
specific callback function for each of the contained items. The callback
function is expected to return either:

  - `CB_CONTINUE` in case iteration shall continue.

  - `CB_BREAK` to abort iteration.

This is needlessly restrictive though, as callers may want to return
arbitrary values and have them be bubbled up to the `cb_each()` call
site. In fact, this is a rather common pattern we have: whenever such a
callback function returns a non-zero error code, we abort iteration and
bubble up the code as-is.

Refactor both the crit-bit tree and oidtree subsystems to behave
accordingly.

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

diff --git a/cbtree.c b/cbtree.c
index cf8cf75b895e953d7c037680be8da1b94d116e07..4ab794bddce0c68f4298151ee714b8296d6e6687 100644 (file)
--- a/cbtree.c
+++ b/cbtree.c
@@ -96,26 +96,28 @@ struct cb_node *cb_lookup(struct cb_tree *t, const uint8_t *k, size_t klen)
        return p && !memcmp(p->k, k, klen) ? p : NULL;
 }
 
-static enum cb_next cb_descend(struct cb_node *p, cb_iter fn, void *arg)
+static int cb_descend(struct cb_node *p, cb_iter fn, void *arg)
 {
        if (1 & (uintptr_t)p) {
                struct cb_node *q = cb_node_of(p);
-               enum cb_next n = cb_descend(q->child[0], fn, arg);
-
-               return n == CB_BREAK ? n : cb_descend(q->child[1], fn, arg);
+               int ret = cb_descend(q->child[0], fn, arg);
+               if (ret)
+                       return ret;
+               return cb_descend(q->child[1], fn, arg);
        } else {
                return fn(p, arg);
        }
 }
 
-void cb_each(struct cb_tree *t, const uint8_t *kpfx, size_t klen,
-                       cb_iter fn, void *arg)
+int cb_each(struct cb_tree *t, const uint8_t *kpfx, size_t klen,
+           cb_iter fn, void *arg)
 {
        struct cb_node *p = t->root;
        struct cb_node *top = p;
        size_t i = 0;
 
-       if (!p) return; /* empty tree */
+       if (!p)
+               return 0; /* empty tree */
 
        /* Walk tree, maintaining top pointer */
        while (1 & (uintptr_t)p) {
@@ -130,7 +132,8 @@ void cb_each(struct cb_tree *t, const uint8_t *kpfx, size_t klen,
 
        for (i = 0; i < klen; i++) {
                if (p->k[i] != kpfx[i])
-                       return; /* "best" match failed */
+                       return 0; /* "best" match failed */
        }
-       cb_descend(top, fn, arg);
+
+       return cb_descend(top, fn, arg);
 }

diff --git a/cbtree.h b/cbtree.h
index 43193abdda23cef279f9e9cee99370509611f7c5..c374b1b3db9d828827120fce55132959ae4b029d 100644 (file)
--- a/cbtree.h
+++ b/cbtree.h
@@ -30,11 +30,6 @@ struct cb_tree {
        struct cb_node *root;
 };
 
-enum cb_next {
-       CB_CONTINUE = 0,
-       CB_BREAK = 1
-};
-
 #define CBTREE_INIT { 0 }
 
 static inline void cb_init(struct cb_tree *t)
@@ -46,9 +41,15 @@ static inline void cb_init(struct cb_tree *t)
 struct cb_node *cb_lookup(struct cb_tree *, const uint8_t *k, size_t klen);
 struct cb_node *cb_insert(struct cb_tree *, struct cb_node *, size_t klen);
 
-typedef enum cb_next (*cb_iter)(struct cb_node *, void *arg);
+/*
+ * Callback invoked by `cb_each()` for each node in the critbit tree. A return
+ * value of 0 will cause the iteration to continue, a non-zero return code will
+ * cause iteration to abort. The error code will be relayed back from
+ * `cb_each()` in that case.
+ */
+typedef int (*cb_iter)(struct cb_node *, void *arg);
 
-void cb_each(struct cb_tree *, const uint8_t *kpfx, size_t klen,
-               cb_iter, void *arg);
+int cb_each(struct cb_tree *, const uint8_t *kpfx, size_t klen,
+           cb_iter, void *arg);
 
 #endif /* CBTREE_H */

diff --git a/object-name.c b/object-name.c
index e5adec4c9d5084d0a947322ba044ce7302b10cef..a24a1b48e12ae91716d7048ceebe68c994b4c098 100644 (file)
--- a/object-name.c
+++ b/object-name.c
@@ -103,12 +103,12 @@ static void update_candidates(struct disambiguate_state *ds, const struct object
 
 static int match_hash(unsigned, const unsigned char *, const unsigned char *);
 
-static enum cb_next match_prefix(const struct object_id *oid, void *arg)
+static int match_prefix(const struct object_id *oid, void *arg)
 {
        struct disambiguate_state *ds = arg;
        /* no need to call match_hash, oidtree_each did prefix match */
        update_candidates(ds, oid);
-       return ds->ambiguous ? CB_BREAK : CB_CONTINUE;
+       return ds->ambiguous;
 }
 
 static void find_short_object_filename(struct disambiguate_state *ds)

diff --git a/oidtree.c b/oidtree.c
index a4d10cd429c9085093ebda7d37e69c646c93f37b..ab9fe7ec7aecce5f457e55aa665c60adbfd606ea 100644 (file)
--- a/oidtree.c
+++ b/oidtree.c
@@ -71,7 +71,7 @@ struct oidtree_each_data {
        uint8_t last_byte;
 };
 
-static enum cb_next iter(struct cb_node *n, void *cb_data)
+static int iter(struct cb_node *n, void *cb_data)
 {
        struct oidtree_each_data *data = cb_data;
        struct object_id k;
@@ -80,18 +80,18 @@ static enum cb_next iter(struct cb_node *n, void *cb_data)
        memcpy(&k, n->k, sizeof(k));
 
        if (data->algo != GIT_HASH_UNKNOWN && data->algo != k.algo)
-               return CB_CONTINUE;
+               return 0;
 
        if (data->last_nibble_at) {
                if ((k.hash[*data->last_nibble_at] ^ data->last_byte) & 0xf0)
-                       return CB_CONTINUE;
+                       return 0;
        }
 
        return data->cb(&k, data->cb_data);
 }
 
-void oidtree_each(struct oidtree *ot, const struct object_id *prefix,
-                 size_t prefix_hex_len, oidtree_each_cb cb, void *cb_data)
+int oidtree_each(struct oidtree *ot, const struct object_id *prefix,
+                size_t prefix_hex_len, oidtree_each_cb cb, void *cb_data)
 {
        struct oidtree_each_data data = {
                .cb = cb,
@@ -106,5 +106,5 @@ void oidtree_each(struct oidtree *ot, const struct object_id *prefix,
                data.last_nibble_at = &klen;
        }
 
-       cb_each(&ot->tree, prefix->hash, klen, iter, &data);
+       return cb_each(&ot->tree, prefix->hash, klen, iter, &data);
 }

diff --git a/oidtree.h b/oidtree.h
index 06514010176beba8f77b2971e97c09c1dc6dbfbe..2b7bad2e60a51d4910e8773d4e28d32f99df1c40 100644 (file)
--- a/oidtree.h
+++ b/oidtree.h
@@ -35,16 +35,22 @@ void oidtree_insert(struct oidtree *ot, const struct object_id *oid);
 /* Check whether the tree contains the given object ID. */
 bool oidtree_contains(struct oidtree *ot, const struct object_id *oid);
 
-/* Callback function used for `oidtree_each()`. */
-typedef enum cb_next (*oidtree_each_cb)(const struct object_id *oid,
-                                       void *cb_data);
+/*
+ * Callback function used for `oidtree_each()`. Returning a non-zero exit code
+ * will cause iteration to stop. The exit code will be propagated to the caller
+ * of `oidtree_each()`.
+ */
+typedef int (*oidtree_each_cb)(const struct object_id *oid,
+                              void *cb_data);
 
 /*
  * Iterate through all object IDs in the tree whose prefix matches the given
  * object ID prefix and invoke the callback function on each of them.
+ *
+ * Returns any non-zero exit code from the provided callback function.
  */
-void oidtree_each(struct oidtree *ot,
-                 const struct object_id *prefix, size_t prefix_hex_len,
-                 oidtree_each_cb cb, void *cb_data);
+int oidtree_each(struct oidtree *ot,
+                const struct object_id *prefix, size_t prefix_hex_len,
+                oidtree_each_cb cb, void *cb_data);
 
 #endif /* OIDTREE_H */

diff --git a/t/unit-tests/u-oidtree.c b/t/unit-tests/u-oidtree.c
index def47c67953bcf69c99ab13369ef545ec180c754..d4d05c7dc3e4f7755e1d404e40e7c1ffb4432ca4 100644 (file)
--- a/t/unit-tests/u-oidtree.c
+++ b/t/unit-tests/u-oidtree.c
@@ -38,7 +38,7 @@ struct expected_hex_iter {
        const char *query;
 };
 
-static enum cb_next check_each_cb(const struct object_id *oid, void *data)
+static int check_each_cb(const struct object_id *oid, void *data)
 {
        struct expected_hex_iter *hex_iter = data;
        struct object_id expected;
@@ -49,7 +49,7 @@ static enum cb_next check_each_cb(const struct object_id *oid, void *data)
                         &expected);
        cl_assert_equal_s(oid_to_hex(oid), oid_to_hex(&expected));
        hex_iter->i += 1;
-       return CB_CONTINUE;
+       return 0;
 }
 
 LAST_ARG_MUST_BE_NULL