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>

Patrick Steinhardt committed Oct 2, 2024 at 12:55 UTC 802c0646ac3c04a16adafde5e7cf899f5fc46821
9 files changed +131 -64
refs/reftable-backend.c
+31 -8
@@ -1307,7 +1307,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
1307 struct reftable_log_record log = {0};
1308 struct reftable_iterator it = {0};
1309
1310 - reftable_stack_init_log_iterator(arg->stack, &it);
1310 + ret = reftable_stack_init_log_iterator(arg->stack, &it);
1311 + if (ret < 0)
1312 + goto done;
1313
1314 /*
1315 * 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)
1679 * copy over all log entries from the old reflog. Last but not least,
1680 * when renaming we also have to delete all the old reflog entries.
1681 */
1680 - reftable_stack_init_log_iterator(arg->stack, &it);
1682 + ret = reftable_stack_init_log_iterator(arg->stack, &it);
1683 + if (ret < 0)
1684 + goto done;
1685 +
1686 ret = reftable_iterator_seek_log(&it, arg->oldname);
1687 if (ret < 0)
1688 goto done;
@@ -1898,7 +1903,10 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
1903 if (ret < 0)
1904 goto done;
1905
1901 - reftable_stack_init_log_iterator(stack, &iter->iter);
1906 + ret = reftable_stack_init_log_iterator(stack, &iter->iter);
1907 + if (ret < 0)
1908 + goto done;
1909 +
1910 ret = reftable_iterator_seek_log(&iter->iter, "");
1911 if (ret < 0)
1912 goto done;
@@ -1965,7 +1973,10 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
1973 if (refs->err < 0)
1974 return refs->err;
1975
1968 - reftable_stack_init_log_iterator(stack, &it);
1976 + ret = reftable_stack_init_log_iterator(stack, &it);
1977 + if (ret < 0)
1978 + goto done;
1979 +
1980 ret = reftable_iterator_seek_log(&it, refname);
1981 while (!ret) {
1982 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,
1992 break;
1993 }
1994
1995 +done:
1996 reftable_log_record_release(&log);
1997 reftable_iterator_destroy(&it);
1998 return ret;
@@ -2002,7 +2014,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
2014 if (refs->err < 0)
2015 return refs->err;
2016
2005 - reftable_stack_init_log_iterator(stack, &it);
2017 + ret = reftable_stack_init_log_iterator(stack, &it);
2018 + if (ret < 0)
2019 + goto done;
2020 +
2021 ret = reftable_iterator_seek_log(&it, refname);
2022 while (!ret) {
2023 struct reftable_log_record log = {0};
@@ -2052,7 +2067,10 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,
2067 if (ret < 0)
2068 goto done;
2069
2055 - reftable_stack_init_log_iterator(stack, &it);
2070 + ret = reftable_stack_init_log_iterator(stack, &it);
2071 + if (ret < 0)
2072 + goto done;
2073 +
2074 ret = reftable_iterator_seek_log(&it, refname);
2075 if (ret < 0)
2076 goto done;
@@ -2158,7 +2176,9 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
2176
2177 reftable_writer_set_limits(writer, ts, ts);
2178
2161 - reftable_stack_init_log_iterator(arg->stack, &it);
2179 + ret = reftable_stack_init_log_iterator(arg->stack, &it);
2180 + if (ret < 0)
2181 + goto out;
2182
2183 /*
2184 * 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
2202 ret = reftable_writer_add_log(writer, &tombstone);
2203 }
2204
2205 +out:
2206 reftable_log_record_release(&log);
2207 reftable_iterator_destroy(&it);
2208 return ret;
@@ -2320,7 +2341,9 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
2341 if (ret < 0)
2342 goto done;
2343
2323 - reftable_stack_init_log_iterator(stack, &it);
2344 + ret = reftable_stack_init_log_iterator(stack, &it);
2345 + if (ret < 0)
2346 + goto done;
2347
2348 ret = reftable_iterator_seek_log(&it, refname);
2349 if (ret < 0)
reftable/merged.c
+47 -27
@@ -30,22 +30,6 @@ struct merged_iter {
30 ssize_t advance_index;
31 };
32
33 -static void merged_iter_init(struct merged_iter *mi,
34 - struct reftable_merged_table *mt,
35 - uint8_t typ)
36 -{
37 - memset(mi, 0, sizeof(*mi));
38 - mi->advance_index = -1;
39 - mi->suppress_deletions = mt->suppress_deletions;
40 -
41 - REFTABLE_CALLOC_ARRAY(mi->subiters, mt->readers_len);
42 - for (size_t i = 0; i < mt->readers_len; i++) {
43 - reftable_record_init(&mi->subiters[i].rec, typ);
44 - reader_init_iter(mt->readers[i], &mi->subiters[i].iter, typ);
45 - }
46 - mi->subiters_len = mt->readers_len;
47 -}
48 -
33 static void merged_iter_close(void *p)
34 {
35 struct merged_iter *mi = p;
@@ -244,25 +228,61 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt)
228 return mt->min;
229 }
230
247 -void merged_table_init_iter(struct reftable_merged_table *mt,
248 - struct reftable_iterator *it,
249 - uint8_t typ)
231 +int merged_table_init_iter(struct reftable_merged_table *mt,
232 + struct reftable_iterator *it,
233 + uint8_t typ)
234 {
251 - struct merged_iter *mi = reftable_malloc(sizeof(*mi));
252 - merged_iter_init(mi, mt, typ);
235 + struct merged_subiter *subiters;
236 + struct merged_iter *mi = NULL;
237 + int ret;
238 +
239 + REFTABLE_CALLOC_ARRAY(subiters, mt->readers_len);
240 + if (!subiters) {
241 + ret = REFTABLE_OUT_OF_MEMORY_ERROR;
242 + goto out;
243 + }
244 +
245 + for (size_t i = 0; i < mt->readers_len; i++) {
246 + reftable_record_init(&subiters[i].rec, typ);
247 + reader_init_iter(mt->readers[i], &subiters[i].iter, typ);
248 + }
249 +
250 + REFTABLE_CALLOC_ARRAY(mi, 1);
251 + if (!mi) {
252 + ret = REFTABLE_OUT_OF_MEMORY_ERROR;
253 + goto out;
254 + }
255 + mi->advance_index = -1;
256 + mi->suppress_deletions = mt->suppress_deletions;
257 + mi->subiters = subiters;
258 + mi->subiters_len = mt->readers_len;
259 +
260 iterator_from_merged_iter(it, mi);
261 + ret = 0;
262 +
263 +out:
264 + if (ret < 0) {
265 + for (size_t i = 0; subiters && i < mt->readers_len; i++) {
266 + reftable_iterator_destroy(&subiters[i].iter);
267 + reftable_record_release(&subiters[i].rec);
268 + }
269 + reftable_free(subiters);
270 + reftable_free(mi);
271 + }
272 +
273 + return ret;
274 }
275
256 -void reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
257 - struct reftable_iterator *it)
276 +int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
277 + struct reftable_iterator *it)
278 {
259 - merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
279 + return merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
280 }
281
262 -void reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
263 - struct reftable_iterator *it)
282 +int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
283 + struct reftable_iterator *it)
284 {
265 - merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
285 + return merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
286 }
287
288 uint32_t reftable_merged_table_hash_id(struct reftable_merged_table *mt)
reftable/merged.h
+3 -3
@@ -26,8 +26,8 @@ struct reftable_merged_table {
26
27 struct reftable_iterator;
28
29 -void merged_table_init_iter(struct reftable_merged_table *mt,
30 - struct reftable_iterator *it,
31 - uint8_t typ);
29 +int merged_table_init_iter(struct reftable_merged_table *mt,
30 + struct reftable_iterator *it,
31 + uint8_t typ);
32
33 #endif
reftable/reftable-merged.h
+4 -4
@@ -37,12 +37,12 @@ int reftable_merged_table_new(struct reftable_merged_table **dest,
37 uint32_t hash_id);
38
39 /* Initialize a merged table iterator for reading refs. */
40 -void reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
41 - struct reftable_iterator *it);
40 +int reftable_merged_table_init_ref_iterator(struct reftable_merged_table *mt,
41 + struct reftable_iterator *it);
42
43 /* Initialize a merged table iterator for reading logs. */
44 -void reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
45 - struct reftable_iterator *it);
44 +int reftable_merged_table_init_log_iterator(struct reftable_merged_table *mt,
45 + struct reftable_iterator *it);
46
47 /* returns the max update_index covered by this merged table. */
48 uint64_t
reftable/reftable-stack.h
+4 -4
@@ -73,16 +73,16 @@ struct reftable_iterator;
73 * be used to iterate through refs. The iterator is valid until the next reload
74 * or write.
75 */
76 -void reftable_stack_init_ref_iterator(struct reftable_stack *st,
77 - struct reftable_iterator *it);
76 +int reftable_stack_init_ref_iterator(struct reftable_stack *st,
77 + struct reftable_iterator *it);
78
79 /*
80 * Initialize an iterator for the merged tables contained in the stack that can
81 * be used to iterate through logs. The iterator is valid until the next reload
82 * or write.
83 */
84 -void reftable_stack_init_log_iterator(struct reftable_stack *st,
85 - struct reftable_iterator *it);
84 +int reftable_stack_init_log_iterator(struct reftable_stack *st,
85 + struct reftable_iterator *it);
86
87 /* returns the merged_table for seeking. This table is valid until the
88 * next write or reload, and should not be closed or deleted.
reftable/stack.c
+23 -11
@@ -136,18 +136,18 @@ int read_lines(const char *filename, char ***namesp)
136 return err;
137 }
138
139 -void reftable_stack_init_ref_iterator(struct reftable_stack *st,
139 +int reftable_stack_init_ref_iterator(struct reftable_stack *st,
140 struct reftable_iterator *it)
141 {
142 - merged_table_init_iter(reftable_stack_merged_table(st),
143 - it, BLOCK_TYPE_REF);
142 + return merged_table_init_iter(reftable_stack_merged_table(st),
143 + it, BLOCK_TYPE_REF);
144 }
145
146 -void reftable_stack_init_log_iterator(struct reftable_stack *st,
147 - struct reftable_iterator *it)
146 +int reftable_stack_init_log_iterator(struct reftable_stack *st,
147 + struct reftable_iterator *it)
148 {
149 - merged_table_init_iter(reftable_stack_merged_table(st),
150 - it, BLOCK_TYPE_LOG);
149 + return merged_table_init_iter(reftable_stack_merged_table(st),
150 + it, BLOCK_TYPE_LOG);
151 }
152
153 struct reftable_merged_table *
@@ -952,7 +952,10 @@ static int stack_write_compact(struct reftable_stack *st,
952 if (err < 0)
953 goto done;
954
955 - merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
955 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
956 + if (err < 0)
957 + goto done;
958 +
959 err = reftable_iterator_seek_ref(&it, "");
960 if (err < 0)
961 goto done;
@@ -977,7 +980,10 @@ static int stack_write_compact(struct reftable_stack *st,
980 }
981 reftable_iterator_destroy(&it);
982
980 - merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
983 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
984 + if (err < 0)
985 + goto done;
986 +
987 err = reftable_iterator_seek_log(&it, "");
988 if (err < 0)
989 goto done;
@@ -1496,7 +1502,10 @@ int reftable_stack_read_ref(struct reftable_stack *st, const char *refname,
1502 struct reftable_iterator it = { 0 };
1503 int ret;
1504
1499 - reftable_merged_table_init_ref_iterator(st->merged, &it);
1505 + ret = reftable_merged_table_init_ref_iterator(st->merged, &it);
1506 + if (ret)
1507 + goto out;
1508 +
1509 ret = reftable_iterator_seek_ref(&it, refname);
1510 if (ret)
1511 goto out;
@@ -1523,7 +1532,10 @@ int reftable_stack_read_log(struct reftable_stack *st, const char *refname,
1532 struct reftable_iterator it = {0};
1533 int err;
1534
1526 - reftable_stack_init_log_iterator(st, &it);
1535 + err = reftable_stack_init_log_iterator(st, &it);
1536 + if (err)
1537 + goto done;
1538 +
1539 err = reftable_iterator_seek_log(&it, refname);
1540 if (err)
1541 goto done;
t/helper/test-reftable.c
+8 -2
@@ -28,7 +28,10 @@ static int dump_table(struct reftable_merged_table *mt)
28 const struct git_hash_algo *algop;
29 int err;
30
31 - reftable_merged_table_init_ref_iterator(mt, &it);
31 + err = reftable_merged_table_init_ref_iterator(mt, &it);
32 + if (err < 0)
33 + return err;
34 +
35 err = reftable_iterator_seek_ref(&it, "");
36 if (err < 0)
37 return err;
@@ -63,7 +66,10 @@ static int dump_table(struct reftable_merged_table *mt)
66 reftable_iterator_destroy(&it);
67 reftable_ref_record_release(&ref);
68
66 - reftable_merged_table_init_log_iterator(mt, &it);
69 + err = reftable_merged_table_init_log_iterator(mt, &it);
70 + if (err < 0)
71 + return err;
72 +
73 err = reftable_iterator_seek_log(&it, "");
74 if (err < 0)
75 return err;
t/unit-tests/t-reftable-merged.c
+8 -4
@@ -82,7 +82,8 @@ static void t_merged_single_record(void)
82 struct reftable_iterator it = { 0 };
83 int err;
84
85 - merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
85 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
86 + check(!err);
87 err = reftable_iterator_seek_ref(&it, "a");
88 check(!err);
89
@@ -161,7 +162,8 @@ static void t_merged_refs(void)
162 size_t cap = 0;
163 size_t i;
164
164 - merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
165 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
166 + check(!err);
167 err = reftable_iterator_seek_ref(&it, "a");
168 check(!err);
169 check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -367,7 +369,8 @@ static void t_merged_logs(void)
369 size_t cap = 0;
370 size_t i;
371
370 - merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
372 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
373 + check(!err);
374 err = reftable_iterator_seek_log(&it, "a");
375 check(!err);
376 check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -390,7 +393,8 @@ static void t_merged_logs(void)
393 check(reftable_log_record_equal(want[i], &out[i],
394 GIT_SHA1_RAWSZ));
395
393 - merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
396 + err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
397 + check(!err);
398 err = reftable_iterator_seek_log_at(&it, "a", 2);
399 check(!err);
400 reftable_log_record_release(&out[0]);
t/unit-tests/t-reftable-stack.c
+3 -1
@@ -599,7 +599,9 @@ static void t_reftable_stack_iterator(void)
599
600 reftable_iterator_destroy(&it);
601
602 - reftable_stack_init_log_iterator(st, &it);
602 + err = reftable_stack_init_log_iterator(st, &it);
603 + check(!err);
604 +
605 reftable_iterator_seek_log(&it, logs[0].refname);
606 for (i = 0; ; i++) {
607 struct reftable_log_record log = { 0 };