From d26c21483d327fdecb52a98be8239f0f224423f9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 3 Jan 2024 07:22:13 +0100 Subject: reftable/stack: do not overwrite errors when compacting In order to compact multiple stacks we iterate through the merged ref and log records. When there is any error either when reading the records from the old merged table or when writing the records to the new table then we break out of the respective loops. When breaking out of the loop for the ref records though the error code will be overwritten, which may cause us to inadvertently skip over bad ref records. In the worst case, this can lead to a compacted stack that is missing records. Fix the code by using `goto done` instead so that any potential error codes are properly returned to the caller. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'reftable/stack.c') diff --git a/reftable/stack.c b/reftable/stack.c index 16bab82063..8729508dc3 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -801,18 +801,16 @@ static int stack_write_compact(struct reftable_stack *st, err = 0; break; } - if (err < 0) { - break; - } + if (err < 0) + goto done; if (first == 0 && reftable_ref_record_is_deletion(&ref)) { continue; } err = reftable_writer_add_ref(wr, &ref); - if (err < 0) { - break; - } + if (err < 0) + goto done; entries++; } reftable_iterator_destroy(&it); @@ -827,9 +825,8 @@ static int stack_write_compact(struct reftable_stack *st, err = 0; break; } - if (err < 0) { - break; - } + if (err < 0) + goto done; if (first == 0 && reftable_log_record_is_deletion(&log)) { continue; } @@ -845,9 +842,8 @@ static int stack_write_compact(struct reftable_stack *st, } err = reftable_writer_add_log(wr, &log); - if (err < 0) { - break; - } + if (err < 0) + goto done; entries++; } -- cgit v1.3 From 75d790608f1e57602a36e29591d356953da26857 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 3 Jan 2024 07:22:17 +0100 Subject: reftable/stack: do not auto-compact twice in `reftable_stack_add()` In 5c086453ff (reftable/stack: perform auto-compaction with transactional interface, 2023-12-11), we fixed a bug where the transactional interface to add changes to a reftable stack did not perform auto-compaction by calling `reftable_stack_auto_compact()` in `reftable_stack_addition_commit()`. While correct, this change may now cause us to perform auto-compaction twice in the non-transactional interface `reftable_stack_add()`: - It performs auto-compaction by itself. - It now transitively performs auto-compaction via the transactional interface. Remove the first instance so that we only end up doing auto-compaction once. Reported-by: Han-Wen Nienhuys Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- reftable/stack.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'reftable/stack.c') diff --git a/reftable/stack.c b/reftable/stack.c index 8729508dc3..7ffeb3ee10 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -425,9 +425,6 @@ int reftable_stack_add(struct reftable_stack *st, return err; } - if (!st->disable_auto_compact) - return reftable_stack_auto_compact(st); - return 0; } -- cgit v1.3