reftable/merged: handle allocation failures in `merged_table_init_iter()`

Handle allocation failures in `merged_table_init_iter()`. While at it,
merge `merged_iter_init()` into the function. It only has a single
caller and merging them makes it easier to handle allocation failures
consistently.

This change also requires us to adapt `reftable_stack_init_*_iterator()`
to bubble up the new error codes of `merged_table_iter_init()`. Adapt
callsites accordingly.

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

diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 3e63833ce4159b8eaf41351b56d14c33313c16c4..9c2c695b134cb33b3b69d95b17968477d4045c5b 100644 (file)
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1307,7 +1307,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
                        struct reftable_log_record log = {0};
                        struct reftable_iterator it = {0};
 
-                       reftable_stack_init_log_iterator(arg->stack, &it);
+                       ret = reftable_stack_init_log_iterator(arg->stack, &it);
+                       if (ret < 0)
+                               goto done;
 
                        /*
                         * When deleting refs we also delete all reflog entries
@@ -1677,7 +1679,10 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
         * copy over all log entries from the old reflog. Last but not least,
         * when renaming we also have to delete all the old reflog entries.
         */
-       reftable_stack_init_log_iterator(arg->stack, &it);
+       ret = reftable_stack_init_log_iterator(arg->stack, &it);
+       if (ret < 0)
+               goto done;
+
        ret = reftable_iterator_seek_log(&it, arg->oldname);
        if (ret < 0)
                goto done;
@@ -1898,7 +1903,10 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
        if (ret < 0)
                goto done;
 
-       reftable_stack_init_log_iterator(stack, &iter->iter);
+       ret = reftable_stack_init_log_iterator(stack, &iter->iter);
+       if (ret < 0)
+               goto done;
+
        ret = reftable_iterator_seek_log(&iter->iter, "");
        if (ret < 0)
                goto done;
@@ -1965,7 +1973,10 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
        if (refs->err < 0)
                return refs->err;
 
-       reftable_stack_init_log_iterator(stack, &it);
+       ret = reftable_stack_init_log_iterator(stack, &it);
+       if (ret < 0)
+               goto done;
+
        ret = reftable_iterator_seek_log(&it, refname);
        while (!ret) {
                ret = reftable_iterator_next_log(&it, &log);
@@ -1981,6 +1992,7 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
                        break;
        }
 
+done:
        reftable_log_record_release(&log);
        reftable_iterator_destroy(&it);
        return ret;
@@ -2002,7 +2014,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
        if (refs->err < 0)
                return refs->err;
 
-       reftable_stack_init_log_iterator(stack, &it);
+       ret = reftable_stack_init_log_iterator(stack, &it);
+       if (ret < 0)
+               goto done;
+
        ret = reftable_iterator_seek_log(&it, refname);
        while (!ret) {
                struct reftable_log_record log = {0};
@@ -2052,7 +2067,10 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,
        if (ret < 0)
                goto done;
 
-       reftable_stack_init_log_iterator(stack, &it);
+       ret = reftable_stack_init_log_iterator(stack, &it);
+       if (ret < 0)
+               goto done;
+
        ret = reftable_iterator_seek_log(&it, refname);
        if (ret < 0)
                goto done;
@@ -2158,7 +2176,9 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
 
        reftable_writer_set_limits(writer, ts, ts);
 
-       reftable_stack_init_log_iterator(arg->stack, &it);
+       ret = reftable_stack_init_log_iterator(arg->stack, &it);
+       if (ret < 0)
+               goto out;
 
        /*
         * In order to delete a table we need to delete all reflog entries one
@@ -2182,6 +2202,7 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
                ret = reftable_writer_add_log(writer, &tombstone);
        }
 
+out:
        reftable_log_record_release(&log);
        reftable_iterator_destroy(&it);
        return ret;
@@ -2320,7 +2341,9 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
        if (ret < 0)
                goto done;
 
-       reftable_stack_init_log_iterator(stack, &it);
+       ret = reftable_stack_init_log_iterator(stack, &it);
+       if (ret < 0)
+               goto done;
 
        ret = reftable_iterator_seek_log(&it, refname);
        if (ret < 0)

diff --git a/reftable/merged.c b/reftable/merged.c
index 128a810c55dd079317107bce90d287679f525394..de4f81abaf5184e73297762623ade7f784306106 100644 (file)
--- a/reftable/merged.c
+++ b/reftable/merged.c
@@ -30,22 +30,6 @@ struct merged_iter {
        ssize_t advance_index;
 };
 
-static void merged_iter_init(struct merged_iter *mi,
-                            struct reftable_merged_table *mt,
-                            uint8_t typ)
-{
-       memset(mi, 0, sizeof(*mi));
-       mi->advance_index = -1;
-       mi->suppress_deletions = mt->suppress_deletions;
-
-       REFTABLE_CALLOC_ARRAY(mi->subiters, mt->readers_len);
-       for (size_t i = 0; i < mt->readers_len; i++) {
-               reftable_record_init(&mi->subiters[i].rec, typ);
-               reader_init_iter(mt->readers[i], &mi->subiters[i].iter, typ);
-       }
-       mi->subiters_len = mt->readers_len;
-}
-
 static void merged_iter_close(void *p)
 {
        struct merged_iter *mi = p;
@@ -244,25 +228,61 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt)
        return mt->min;
 }
 
-void merged_table_init_iter(struct reftable_merged_table *mt,
-                           struct reftable_iterator *it,
-                           uint8_t typ)
+int merged_table_init_iter(struct reftable_merged_table *mt,
+                          struct reftable_iterator *it,
+                          uint8_t typ)
 {
-       struct merged_iter *mi = reftable_malloc(sizeof(*mi));
-       merged_iter_init(mi, mt, typ);
+       struct merged_subiter *subiters;
+       struct merged_iter *mi = NULL;
+       int ret;
+
+       REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len);
+       if (!subiters) {
+               ret = REFTABLE_OUT_OF_MEMORY_ERROR;
+               goto out;
+       }
+
+       for (size_t i = 0; i < mt->readers_len; i++) {
+               reftable_record_init(&subiters[i].rec, typ);
+               reader_init_iter(mt->readers[i], &subiters[i].iter, typ);
+       }
+
+       REFTABLE_CALLOC_ARRAY(mi, 1);
+       if (!mi) {
+               ret = REFTABLE_OUT_OF_MEMORY_ERROR;
+               goto out;
+       }
+       mi->advance_index = -1;
+       mi->suppress_deletions = mt->suppress_deletions;
+       mi->subiters = subiters;
+       mi->subiters_len = mt->readers_len;
+
        iterator_from_merged_iter(it, mi);
+       ret = 0;
+
+out:
+       if (ret < 0) {
+               for (size_t i = 0; subiters && i < mt->readers_len; i++) {
+                       reftable_iterator_destroy(&subiters[i].iter);
+                       reftable_record_release(&subiters[i].rec);
+               }
+               reftable_free(subiters);
+               reftable_free(mi);
+       }
+
+       return ret;
 }
 
-void reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
-                                            struct reftable_iterator *it)
+int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
+                                           struct reftable_iterator *it)
 {
-       merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
+       return merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
 }
 
-void reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
-                                            struct reftable_iterator *it)
+int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
+                                           struct reftable_iterator *it)
 {
-       merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
+       return merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
 }
 
 uint32_t reftable_merged_table_hash_id(struct reftable_merged_table *mt)

diff --git a/reftable/merged.h b/reftable/merged.h
index de5fd33f010b1bf7f5c9435b0199fa2b0cc7d3b6..89bd0c4b35b8e64600cb594c1989d68874037f3d 100644 (file)
--- a/reftable/merged.h
+++ b/reftable/merged.h
@@ -26,8 +26,8 @@ struct reftable_merged_table {
 
 struct reftable_iterator;
 
-void merged_table_init_iter(struct reftable_merged_table *mt,
-                           struct reftable_iterator *it,
-                           uint8_t typ);
+int merged_table_init_iter(struct reftable_merged_table *mt,
+                          struct reftable_iterator *it,
+                          uint8_t typ);
 
 #endif

diff --git a/reftable/reftable-merged.h b/reftable/reftable-merged.h
index 16d19f8df206f3540cb471a12fa9060e2e8863f8..a970d5dd89ac34b20d51e89ca41cffd5a60026aa 100644 (file)
--- a/reftable/reftable-merged.h
+++ b/reftable/reftable-merged.h
@@ -37,12 +37,12 @@ int reftable_merged_table_new(struct reftable_merged_table **dest,
                              uint32_t hash_id);
 
 /* Initialize a merged table iterator for reading refs. */
-void reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
-                                            struct reftable_iterator *it);
+int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
+                                           struct reftable_iterator *it);
 
 /* Initialize a merged table iterator for reading logs. */
-void reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
-                                            struct reftable_iterator *it);
+int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
+                                           struct reftable_iterator *it);
 
 /* returns the max update_index covered by this merged table. */
 uint64_t

diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h
index f4f8cabc7fb560ae3dc18882879b4aedc4076d83..e958f911b4e691b4b6367a1975e696dcfd67d993 100644 (file)
--- a/reftable/reftable-stack.h
+++ b/reftable/reftable-stack.h
@@ -73,16 +73,16 @@ struct reftable_iterator;
  * be used to iterate through refs. The iterator is valid until the next reload
  * or write.
  */
-void reftable_stack_init_ref_iterator(struct reftable_stack *st,
-                                     struct reftable_iterator *it);
+int reftable_stack_init_ref_iterator(struct reftable_stack *st,
+                                    struct reftable_iterator *it);
 
 /*
  * Initialize an iterator for the merged tables contained in the stack that can
  * be used to iterate through logs. The iterator is valid until the next reload
  * or write.
  */
-void reftable_stack_init_log_iterator(struct reftable_stack *st,
-                                     struct reftable_iterator *it);
+int reftable_stack_init_log_iterator(struct reftable_stack *st,
+                                    struct reftable_iterator *it);
 
 /* returns the merged_table for seeking. This table is valid until the
  * next write or reload, and should not be closed or deleted.

diff --git a/reftable/stack.c b/reftable/stack.c
index ea21ca6e5f546fb1ed73a4bbd0762d3bcf74820b..bb4d230918963d6b71dcfc83a3bea8be88751853 100644 (file)
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -136,18 +136,18 @@ int read_lines(const char *filename, char ***namesp)
        return err;
 }
 
-void reftable_stack_init_ref_iterator(struct reftable_stack *st,
+int reftable_stack_init_ref_iterator(struct reftable_stack *st,
                                      struct reftable_iterator *it)
 {
-       merged_table_init_iter(reftable_stack_merged_table(st),
-                              it, BLOCK_TYPE_REF);
+       return merged_table_init_iter(reftable_stack_merged_table(st),
+                                     it, BLOCK_TYPE_REF);
 }
 
-void reftable_stack_init_log_iterator(struct reftable_stack *st,
-                                     struct reftable_iterator *it)
+int reftable_stack_init_log_iterator(struct reftable_stack *st,
+                                    struct reftable_iterator *it)
 {
-       merged_table_init_iter(reftable_stack_merged_table(st),
-                              it, BLOCK_TYPE_LOG);
+       return merged_table_init_iter(reftable_stack_merged_table(st),
+                                     it, BLOCK_TYPE_LOG);
 }
 
 struct reftable_merged_table *
@@ -952,7 +952,10 @@ static int stack_write_compact(struct reftable_stack *st,
        if (err < 0)
                goto done;
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       if (err < 0)
+               goto done;
+
        err = reftable_iterator_seek_ref(&it, "");
        if (err < 0)
                goto done;
@@ -977,7 +980,10 @@ static int stack_write_compact(struct reftable_stack *st,
        }
        reftable_iterator_destroy(&it);
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       if (err < 0)
+               goto done;
+
        err = reftable_iterator_seek_log(&it, "");
        if (err < 0)
                goto done;
@@ -1496,7 +1502,10 @@ int reftable_stack_read_ref(struct reftable_stack *st, const char *refname,
        struct reftable_iterator it = { 0 };
        int ret;
 
-       reftable_merged_table_init_ref_iterator(st->merged, &it);
+       ret = reftable_merged_table_init_ref_iterator(st->merged, &it);
+       if (ret)
+               goto out;
+
        ret = reftable_iterator_seek_ref(&it, refname);
        if (ret)
                goto out;
@@ -1523,7 +1532,10 @@ int reftable_stack_read_log(struct reftable_stack *st, const char *refname,
        struct reftable_iterator it = {0};
        int err;
 
-       reftable_stack_init_log_iterator(st, &it);
+       err = reftable_stack_init_log_iterator(st, &it);
+       if (err)
+               goto done;
+
        err = reftable_iterator_seek_log(&it, refname);
        if (err)
                goto done;

diff --git a/t/helper/test-reftable.c b/t/helper/test-reftable.c
index 29d4e9a755d6de28980051acbd1c3d16a93a6134..5c8849d115b5a30b8dccdeded3847cca787824d7 100644 (file)
--- a/t/helper/test-reftable.c
+++ b/t/helper/test-reftable.c
@@ -28,7 +28,10 @@ static int dump_table(struct reftable_merged_table *mt)
        const struct git_hash_algo *algop;
        int err;
 
-       reftable_merged_table_init_ref_iterator(mt, &it);
+       err = reftable_merged_table_init_ref_iterator(mt, &it);
+       if (err < 0)
+               return err;
+
        err = reftable_iterator_seek_ref(&it, "");
        if (err < 0)
                return err;
@@ -63,7 +66,10 @@ static int dump_table(struct reftable_merged_table *mt)
        reftable_iterator_destroy(&it);
        reftable_ref_record_release(&ref);
 
-       reftable_merged_table_init_log_iterator(mt, &it);
+       err = reftable_merged_table_init_log_iterator(mt, &it);
+       if (err < 0)
+               return err;
+
        err = reftable_iterator_seek_log(&it, "");
        if (err < 0)
                return err;

diff --git a/t/unit-tests/t-reftable-merged.c b/t/unit-tests/t-reftable-merged.c
index 19e54bdfb8b5ead3929299220e52d59dd13804a1..3d2848632db2eb816a1c4a214a703af32edbdf51 100644 (file)
--- a/t/unit-tests/t-reftable-merged.c
+++ b/t/unit-tests/t-reftable-merged.c
@@ -82,7 +82,8 @@ static void t_merged_single_record(void)
        struct reftable_iterator it = { 0 };
        int err;
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       check(!err);
        err = reftable_iterator_seek_ref(&it, "a");
        check(!err);
 
@@ -161,7 +162,8 @@ static void t_merged_refs(void)
        size_t cap = 0;
        size_t i;
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
+       check(!err);
        err = reftable_iterator_seek_ref(&it, "a");
        check(!err);
        check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -367,7 +369,8 @@ static void t_merged_logs(void)
        size_t cap = 0;
        size_t i;
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       check(!err);
        err = reftable_iterator_seek_log(&it, "a");
        check(!err);
        check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -390,7 +393,8 @@ static void t_merged_logs(void)
                check(reftable_log_record_equal(want[i], &out[i],
                                                 GIT_SHA1_RAWSZ));
 
-       merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
+       check(!err);
        err = reftable_iterator_seek_log_at(&it, "a", 2);
        check(!err);
        reftable_log_record_release(&out[0]);

diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
index 65e513d5ec829b8f23df2cbcde7e6df3e89e8f1f..6e41a4cf7e57cf647e7c6a0035a37a070c2daa93 100644 (file)
--- a/t/unit-tests/t-reftable-stack.c
+++ b/t/unit-tests/t-reftable-stack.c
@@ -599,7 +599,9 @@ static void t_reftable_stack_iterator(void)
 
        reftable_iterator_destroy(&it);
 
-       reftable_stack_init_log_iterator(st, &it);
+       err = reftable_stack_init_log_iterator(st, &it);
+       check(!err);
+
        reftable_iterator_seek_log(&it, logs[0].refname);
        for (i = 0; ; i++) {
                struct reftable_log_record log = { 0 };