Skip to content

Commit 802c064

Browse files
pks-tgitster
pks-t
authored and
gitster
committed
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 74d1c18 commit 802c064

File tree

9 files changed

+131
-64
lines changed

9 files changed

+131
-64
lines changed

‎refs/reftable-backend.c

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
13071307
struct reftable_log_record log = {0};
13081308
struct reftable_iterator it = {0};
13091309

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;
13111313

13121314
/*
13131315
* 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)
16771679
* copy over all log entries from the old reflog. Last but not least,
16781680
* when renaming we also have to delete all the old reflog entries.
16791681
*/
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+
16811686
ret = reftable_iterator_seek_log(&it, arg->oldname);
16821687
if (ret < 0)
16831688
goto done;
@@ -1898,7 +1903,10 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl
18981903
if (ret < 0)
18991904
goto done;
19001905

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+
19021910
ret = reftable_iterator_seek_log(&iter->iter, "");
19031911
if (ret < 0)
19041912
goto done;
@@ -1965,7 +1973,10 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
19651973
if (refs->err < 0)
19661974
return refs->err;
19671975

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+
19691980
ret = reftable_iterator_seek_log(&it, refname);
19701981
while (!ret) {
19711982
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,
19811992
break;
19821993
}
19831994

1995+
done:
19841996
reftable_log_record_release(&log);
19851997
reftable_iterator_destroy(&it);
19861998
return ret;
@@ -2002,7 +2014,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
20022014
if (refs->err < 0)
20032015
return refs->err;
20042016

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+
20062021
ret = reftable_iterator_seek_log(&it, refname);
20072022
while (!ret) {
20082023
struct reftable_log_record log = {0};
@@ -2052,7 +2067,10 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,
20522067
if (ret < 0)
20532068
goto done;
20542069

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+
20562074
ret = reftable_iterator_seek_log(&it, refname);
20572075
if (ret < 0)
20582076
goto done;
@@ -2158,7 +2176,9 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
21582176

21592177
reftable_writer_set_limits(writer, ts, ts);
21602178

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;
21622182

21632183
/*
21642184
* 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
21822202
ret = reftable_writer_add_log(writer, &tombstone);
21832203
}
21842204

2205+
out:
21852206
reftable_log_record_release(&log);
21862207
reftable_iterator_destroy(&it);
21872208
return ret;
@@ -2320,7 +2341,9 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
23202341
if (ret < 0)
23212342
goto done;
23222343

2323-
reftable_stack_init_log_iterator(stack, &it);
2344+
ret = reftable_stack_init_log_iterator(stack, &it);
2345+
if (ret < 0)
2346+
goto done;
23242347

23252348
ret = reftable_iterator_seek_log(&it, refname);
23262349
if (ret < 0)

‎reftable/merged.c

Lines changed: 47 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,6 @@ struct merged_iter {
3030
ssize_t advance_index;
3131
};
3232

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-
4933
static void merged_iter_close(void *p)
5034
{
5135
struct merged_iter *mi = p;
@@ -244,25 +228,61 @@ reftable_merged_table_min_update_index(struct reftable_merged_table *mt)
244228
return mt->min;
245229
}
246230

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)
250234
{
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+
253260
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;
254274
}
255275

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)
258278
{
259-
merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
279+
return merged_table_init_iter(mt, it, BLOCK_TYPE_REF);
260280
}
261281

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)
264284
{
265-
merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
285+
return merged_table_init_iter(mt, it, BLOCK_TYPE_LOG);
266286
}
267287

268288
uint32_t reftable_merged_table_hash_id(struct reftable_merged_table *mt)

‎reftable/merged.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ struct reftable_merged_table {
2626

2727
struct reftable_iterator;
2828

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);
3232

3333
#endif

‎reftable/reftable-merged.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ int reftable_merged_table_new(struct reftable_merged_table **dest,
3737
uint32_t hash_id);
3838

3939
/* 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);
4242

4343
/* 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);
4646

4747
/* returns the max update_index covered by this merged table. */
4848
uint64_t

‎reftable/reftable-stack.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,16 @@ struct reftable_iterator;
7373
* be used to iterate through refs. The iterator is valid until the next reload
7474
* or write.
7575
*/
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);
7878

7979
/*
8080
* Initialize an iterator for the merged tables contained in the stack that can
8181
* be used to iterate through logs. The iterator is valid until the next reload
8282
* or write.
8383
*/
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);
8686

8787
/* returns the merged_table for seeking. This table is valid until the
8888
* next write or reload, and should not be closed or deleted.

‎reftable/stack.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,18 @@ int read_lines(const char *filename, char ***namesp)
136136
return err;
137137
}
138138

139-
void reftable_stack_init_ref_iterator(struct reftable_stack *st,
139+
int reftable_stack_init_ref_iterator(struct reftable_stack *st,
140140
struct reftable_iterator *it)
141141
{
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);
144144
}
145145

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)
148148
{
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);
151151
}
152152

153153
struct reftable_merged_table *
@@ -952,7 +952,10 @@ static int stack_write_compact(struct reftable_stack *st,
952952
if (err < 0)
953953
goto done;
954954

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+
956959
err = reftable_iterator_seek_ref(&it, "");
957960
if (err < 0)
958961
goto done;
@@ -977,7 +980,10 @@ static int stack_write_compact(struct reftable_stack *st,
977980
}
978981
reftable_iterator_destroy(&it);
979982

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+
981987
err = reftable_iterator_seek_log(&it, "");
982988
if (err < 0)
983989
goto done;
@@ -1496,7 +1502,10 @@ int reftable_stack_read_ref(struct reftable_stack *st, const char *refname,
14961502
struct reftable_iterator it = { 0 };
14971503
int ret;
14981504

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+
15001509
ret = reftable_iterator_seek_ref(&it, refname);
15011510
if (ret)
15021511
goto out;
@@ -1523,7 +1532,10 @@ int reftable_stack_read_log(struct reftable_stack *st, const char *refname,
15231532
struct reftable_iterator it = {0};
15241533
int err;
15251534

1526-
reftable_stack_init_log_iterator(st, &it);
1535+
err = reftable_stack_init_log_iterator(st, &it);
1536+
if (err)
1537+
goto done;
1538+
15271539
err = reftable_iterator_seek_log(&it, refname);
15281540
if (err)
15291541
goto done;

‎t/helper/test-reftable.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ static int dump_table(struct reftable_merged_table *mt)
2828
const struct git_hash_algo *algop;
2929
int err;
3030

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+
3235
err = reftable_iterator_seek_ref(&it, "");
3336
if (err < 0)
3437
return err;
@@ -63,7 +66,10 @@ static int dump_table(struct reftable_merged_table *mt)
6366
reftable_iterator_destroy(&it);
6467
reftable_ref_record_release(&ref);
6568

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+
6773
err = reftable_iterator_seek_log(&it, "");
6874
if (err < 0)
6975
return err;

‎t/unit-tests/t-reftable-merged.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ static void t_merged_single_record(void)
8282
struct reftable_iterator it = { 0 };
8383
int err;
8484

85-
merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
85+
err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
86+
check(!err);
8687
err = reftable_iterator_seek_ref(&it, "a");
8788
check(!err);
8889

@@ -161,7 +162,8 @@ static void t_merged_refs(void)
161162
size_t cap = 0;
162163
size_t i;
163164

164-
merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
165+
err = merged_table_init_iter(mt, &it, BLOCK_TYPE_REF);
166+
check(!err);
165167
err = reftable_iterator_seek_ref(&it, "a");
166168
check(!err);
167169
check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -367,7 +369,8 @@ static void t_merged_logs(void)
367369
size_t cap = 0;
368370
size_t i;
369371

370-
merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
372+
err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
373+
check(!err);
371374
err = reftable_iterator_seek_log(&it, "a");
372375
check(!err);
373376
check_int(reftable_merged_table_hash_id(mt), ==, GIT_SHA1_FORMAT_ID);
@@ -390,7 +393,8 @@ static void t_merged_logs(void)
390393
check(reftable_log_record_equal(want[i], &out[i],
391394
GIT_SHA1_RAWSZ));
392395

393-
merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
396+
err = merged_table_init_iter(mt, &it, BLOCK_TYPE_LOG);
397+
check(!err);
394398
err = reftable_iterator_seek_log_at(&it, "a", 2);
395399
check(!err);
396400
reftable_log_record_release(&out[0]);

0 commit comments

Comments
 (0)