reftable/stack: adapt `stack_filename()` to handle allocation failures

The `stack_filename()` function cannot pass any errors to the caller as it has a `void` return type. Adapt it and its callers such that we can handle errors and start handling allocation failures. There are two interesting edge cases in `reftable_stack_destroy()` and `reftable_addition_close()`. Both of these are trying to tear down their respective structures, and while doing so they try to unlink some of the tables they have been keeping alive. Any earlier attempts to do that may fail on Windows because it keeps us from deleting such tables while they are still open, and thus we re-try on close. It's okay and even expected that this can fail when the tables are still open by another process, so we handle the allocation failures gracefully and just skip over any file whose name we couldn't figure out. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Patrick Steinhardt committed Oct 17, 2024 at 06:54 UTC 591c6a600e0ef1bfc71d66d74b64bf47de62fc8e
1 file changed +45 -17
reftable/stack.c
+45 -17
@@ -31,13 +31,16 @@ static void reftable_addition_close(struct reftable_addition *add);
31 static int reftable_stack_reload_maybe_reuse(struct reftable_stack *st,
32 int reuse_open);
33
34 -static void stack_filename(struct reftable_buf *dest, struct reftable_stack *st,
35 - const char *name)
34 +static int stack_filename(struct reftable_buf *dest, struct reftable_stack *st,
35 + const char *name)
36 {
37 + int err;
38 reftable_buf_reset(dest);
38 - reftable_buf_addstr(dest, st->reftable_dir);
39 - reftable_buf_addstr(dest, "/");
40 - reftable_buf_addstr(dest, name);
39 + if ((err = reftable_buf_addstr(dest, st->reftable_dir)) < 0 ||
40 + (err = reftable_buf_addstr(dest, "/")) < 0 ||
41 + (err = reftable_buf_addstr(dest, name)) < 0)
42 + return err;
43 + return 0;
44 }
45
46 static ssize_t reftable_fd_write(void *arg, const void *data, size_t sz)
@@ -211,13 +214,16 @@ void reftable_stack_destroy(struct reftable_stack *st)
214 struct reftable_buf filename = REFTABLE_BUF_INIT;
215 for (i = 0; i < st->readers_len; i++) {
216 const char *name = reader_name(st->readers[i]);
217 + int try_unlinking = 1;
218 +
219 reftable_buf_reset(&filename);
220 if (names && !has_name(names, name)) {
216 - stack_filename(&filename, st, name);
221 + if (stack_filename(&filename, st, name) < 0)
222 + try_unlinking = 0;
223 }
224 reftable_reader_decref(st->readers[i]);
225
220 - if (filename.len) {
226 + if (try_unlinking && filename.len) {
227 /* On Windows, can only unlink after closing. */
228 unlink(filename.buf);
229 }
@@ -310,7 +316,10 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
316
317 if (!rd) {
318 struct reftable_block_source src = { NULL };
313 - stack_filename(&table_path, st, name);
319 +
320 + err = stack_filename(&table_path, st, name);
321 + if (err < 0)
322 + goto done;
323
324 err = reftable_block_source_from_file(&src,
325 table_path.buf);
@@ -341,7 +350,11 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
350 for (i = 0; i < cur_len; i++) {
351 if (cur[i]) {
352 const char *name = reader_name(cur[i]);
344 - stack_filename(&table_path, st, name);
353 +
354 + err = stack_filename(&table_path, st, name);
355 + if (err < 0)
356 + goto done;
357 +
358 reftable_reader_decref(cur[i]);
359 unlink(table_path.buf);
360 }
@@ -700,8 +713,8 @@ static void reftable_addition_close(struct reftable_addition *add)
713 size_t i;
714
715 for (i = 0; i < add->new_tables_len; i++) {
703 - stack_filename(&nm, add->stack, add->new_tables[i]);
704 - unlink(nm.buf);
716 + if (!stack_filename(&nm, add->stack, add->new_tables[i]))
717 + unlink(nm.buf);
718 reftable_free(add->new_tables[i]);
719 add->new_tables[i] = NULL;
720 }
@@ -851,7 +864,9 @@ int reftable_addition_add(struct reftable_addition *add,
864 if (err < 0)
865 goto done;
866
854 - stack_filename(&temp_tab_file_name, add->stack, next_name.buf);
867 + err = stack_filename(&temp_tab_file_name, add->stack, next_name.buf);
868 + if (err < 0)
869 + goto done;
870 reftable_buf_addstr(&temp_tab_file_name, ".temp.XXXXXX");
871
872 tab_file = mks_tempfile(temp_tab_file_name.buf);
@@ -900,7 +915,10 @@ int reftable_addition_add(struct reftable_addition *add,
915 if (err < 0)
916 goto done;
917 reftable_buf_addstr(&next_name, ".ref");
903 - stack_filename(&tab_file_name, add->stack, next_name.buf);
918 +
919 + err = stack_filename(&tab_file_name, add->stack, next_name.buf);
920 + if (err < 0)
921 + goto done;
922
923 /*
924 On windows, this relies on rand() picking a unique destination name.
@@ -954,7 +972,9 @@ static int stack_compact_locked(struct reftable_stack *st,
972 if (err < 0)
973 goto done;
974
957 - stack_filename(&tab_file_path, st, next_name.buf);
975 + err = stack_filename(&tab_file_path, st, next_name.buf);
976 + if (err < 0)
977 + goto done;
978 reftable_buf_addstr(&tab_file_path, ".temp.XXXXXX");
979
980 tab_file = mks_tempfile(tab_file_path.buf);
@@ -1174,7 +1194,9 @@ static int stack_compact_range(struct reftable_stack *st,
1194 }
1195
1196 for (i = last + 1; i > first; i--) {
1177 - stack_filename(&table_name, st, reader_name(st->readers[i - 1]));
1197 + err = stack_filename(&table_name, st, reader_name(st->readers[i - 1]));
1198 + if (err < 0)
1199 + goto done;
1200
1201 err = hold_lock_file_for_update(&table_locks[nlocks],
1202 table_name.buf, LOCK_NO_DEREF);
@@ -1383,7 +1405,10 @@ static int stack_compact_range(struct reftable_stack *st,
1405 goto done;
1406
1407 reftable_buf_addstr(&new_table_name, ".ref");
1386 - stack_filename(&new_table_path, st, new_table_name.buf);
1408 +
1409 + err = stack_filename(&new_table_path, st, new_table_name.buf);
1410 + if (err < 0)
1411 + goto done;
1412
1413 err = rename_tempfile(&new_table, new_table_path.buf);
1414 if (err < 0) {
@@ -1677,7 +1702,10 @@ static void remove_maybe_stale_table(struct reftable_stack *st, uint64_t max,
1702 struct reftable_block_source src = { NULL };
1703 struct reftable_reader *rd = NULL;
1704 struct reftable_buf table_path = REFTABLE_BUF_INIT;
1680 - stack_filename(&table_path, st, name);
1705 +
1706 + err = stack_filename(&table_path, st, name);
1707 + if (err < 0)
1708 + goto done;
1709
1710 err = reftable_block_source_from_file(&src, table_path.buf);
1711 if (err < 0)