oidtree: add ability to store data

The oidtree data structure is currently only used to store object IDs,
without any associated data. So consequently, it can only really be used
to track which object IDs exist, and we can use the tree structure to
efficiently operate on OID prefixes.

But there are valid use cases where we want to both:

  - Store object IDs in a sorted order.

  - Associated arbitrary data with them.

Refactor the oidtree interface so that it allows us to store arbitrary
payloads within the respective nodes. This will be used in the next
commit.

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

diff --git a/loose.c b/loose.c
index 07333be6969fcc370b052fa1ed7c684442ff20af..f7a3dd1a72f0fc32d1f2b42479e609e57dc4baf8 100644 (file)
--- a/loose.c
+++ b/loose.c
@@ -57,7 +57,7 @@ static int insert_loose_map(struct odb_source *source,
        inserted |= insert_oid_pair(map->to_compat, oid, compat_oid);
        inserted |= insert_oid_pair(map->to_storage, compat_oid, oid);
        if (inserted)
-               oidtree_insert(files->loose->cache, compat_oid);
+               oidtree_insert(files->loose->cache, compat_oid, NULL);
 
        return inserted;
 }

diff --git a/object-file.c b/object-file.c
index 7b1a12f8eb6006c39e0c5b542b56a35ccaaccd26..8705251e4d1e2905e388a2d7107e431cf9ba0a05 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -1858,6 +1858,7 @@ static int for_each_object_wrapper_cb(const struct object_id *oid,
 }
 
 static int for_each_prefixed_object_wrapper_cb(const struct object_id *oid,
+                                              void *node_data UNUSED,
                                               void *cb_data)
 {
        struct for_each_object_wrapper_data *data = cb_data;
@@ -2003,7 +2004,7 @@ static int append_loose_object(const struct object_id *oid,
                               const char *path UNUSED,
                               void *data)
 {
-       oidtree_insert(data, oid);
+       oidtree_insert(data, oid, NULL);
        return 0;
 }
 

diff --git a/oidtree.c b/oidtree.c
index 117649753fbc1fa3bae3a1ed42c6f0b262391741..e43f18026e10413a92a5bed50814f6e5621d2e9a 100644 (file)
--- a/oidtree.c
+++ b/oidtree.c
@@ -9,6 +9,7 @@
 struct oidtree_node {
        struct cb_node base;
        struct object_id key;
+       void *data;
 };
 
 void oidtree_init(struct oidtree *ot)
@@ -25,15 +26,22 @@ void oidtree_clear(struct oidtree *ot)
        }
 }
 
-void oidtree_insert(struct oidtree *ot, const struct object_id *oid)
+struct oidtree_data {
+       struct object_id oid;
+};
+
+void oidtree_insert(struct oidtree *ot, const struct object_id *oid,
+                   void *data)
 {
        struct oidtree_node *on;
+       struct cb_node *node;
 
        if (!oid->algo)
                BUG("oidtree_insert requires oid->algo");
 
        on = mem_pool_alloc(&ot->mem_pool, sizeof(*on));
        oidcpy(&on->key, oid);
+       on->data = data;
 
        /*
         * n.b. Current callers won't get us duplicates, here.  If a
@@ -41,13 +49,19 @@ void oidtree_insert(struct oidtree *ot, const struct object_id *oid)
         * that won't be freed until oidtree_clear.  Currently it's not
         * worth maintaining a free list
         */
-       cb_insert(&ot->tree, &on->base, sizeof(*oid));
+       node = cb_insert(&ot->tree, &on->base, sizeof(*oid));
+       if (node) {
+               struct oidtree_node *preexisting = container_of(node, struct oidtree_node, base);
+               preexisting->data = data;
+       }
 }
 
-bool oidtree_contains(struct oidtree *ot, const struct object_id *oid)
+static struct oidtree_node *oidtree_lookup(struct oidtree *ot,
+                                          const struct object_id *oid)
 {
        struct object_id k;
        size_t klen = sizeof(k);
+       struct cb_node *node;
 
        oidcpy(&k, oid);
 
@@ -58,7 +72,20 @@ bool oidtree_contains(struct oidtree *ot, const struct object_id *oid)
        klen += BUILD_ASSERT_OR_ZERO(offsetof(struct object_id, hash) <
                                offsetof(struct object_id, algo));
 
-       return !!cb_lookup(&ot->tree, (const uint8_t *)&k, klen);
+       node = cb_lookup(&ot->tree, (const uint8_t *)&k, klen);
+       return node ? container_of(node, struct oidtree_node, base) : NULL;
+}
+
+bool oidtree_contains(struct oidtree *ot, const struct object_id *oid)
+{
+       struct oidtree_node *node = oidtree_lookup(ot, oid);
+       return node ? 1 : 0;
+}
+
+void *oidtree_get(struct oidtree *ot, const struct object_id *oid)
+{
+       struct oidtree_node *node = oidtree_lookup(ot, oid);
+       return node ? node->data : NULL;
 }
 
 struct oidtree_each_data {
@@ -82,7 +109,7 @@ static int iter(struct cb_node *n, void *cb_data)
                        return 0;
        }
 
-       return data->cb(&node->key, data->cb_data);
+       return data->cb(&node->key, node->data, data->cb_data);
 }
 
 int oidtree_each(struct oidtree *ot, const struct object_id *prefix,

diff --git a/oidtree.h b/oidtree.h
index 2b7bad2e60a51d4910e8773d4e28d32f99df1c40..baa5a436eadaf83f75b5da3775c8388a7563b3a0 100644 (file)
--- a/oidtree.h
+++ b/oidtree.h
@@ -29,18 +29,26 @@ void oidtree_init(struct oidtree *ot);
  */
 void oidtree_clear(struct oidtree *ot);
 
-/* Insert the object ID into the tree. */
-void oidtree_insert(struct oidtree *ot, const struct object_id *oid);
+/*
+ * Insert the object ID into the tree and store the given pointer alongside
+ * with it. The data pointer of any preexisting entry will be overwritten.
+ */
+void oidtree_insert(struct oidtree *ot, const struct object_id *oid,
+                   void *data);
 
 /* Check whether the tree contains the given object ID. */
 bool oidtree_contains(struct oidtree *ot, const struct object_id *oid);
 
+/* Get the payload stored with the given object ID. */
+void *oidtree_get(struct oidtree *ot, const struct object_id *oid);
+
 /*
  * 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 *node_data,
                               void *cb_data);
 
 /*

diff --git a/t/unit-tests/u-oidtree.c b/t/unit-tests/u-oidtree.c
index d4d05c7dc3e4f7755e1d404e40e7c1ffb4432ca4..f0d5ebb733bfcef3394537c440dd048a64224da7 100644 (file)
--- a/t/unit-tests/u-oidtree.c
+++ b/t/unit-tests/u-oidtree.c
@@ -19,7 +19,7 @@ static int fill_tree_loc(struct oidtree *ot, const char *hexes[], size_t n)
        for (size_t i = 0; i < n; i++) {
                struct object_id oid;
                cl_parse_any_oid(hexes[i], &oid);
-               oidtree_insert(ot, &oid);
+               oidtree_insert(ot, &oid, NULL);
        }
        return 0;
 }
@@ -38,9 +38,9 @@ struct expected_hex_iter {
        const char *query;
 };
 
-static int check_each_cb(const struct object_id *oid, void *data)
+static int check_each_cb(const struct object_id *oid, void *node_data UNUSED, void *cb_data)
 {
-       struct expected_hex_iter *hex_iter = data;
+       struct expected_hex_iter *hex_iter = cb_data;
        struct object_id expected;
 
        cl_assert(hex_iter->i < hex_iter->expected_hexes.nr);
@@ -105,3 +105,23 @@ void test_oidtree__each(void)
        check_each(&ot, "32100", "321", NULL);
        check_each(&ot, "32", "320", "321", NULL);
 }
+
+void test_oidtree__insert_overwrites_data(void)
+{
+       struct object_id oid;
+       struct oidtree ot;
+       int a, b;
+
+       cl_parse_any_oid("1", &oid);
+
+       oidtree_init(&ot);
+
+       oidtree_insert(&ot, &oid, NULL);
+       cl_assert_equal_p(oidtree_get(&ot, &oid), NULL);
+       oidtree_insert(&ot, &oid, &a);
+       cl_assert_equal_p(oidtree_get(&ot, &oid), &a);
+       oidtree_insert(&ot, &oid, &b);
+       cl_assert_equal_p(oidtree_get(&ot, &oid), &b);
+
+       oidtree_clear(&ot);
+}