[GSoC PATCH v3 0/2] reftable: return proper error codes from block_writer_add
From: Meet Soni <hidden>
Date: 2025-03-12 12:12:09
This patch attempts to avoid making an assumption regarding error codes
returned by block_writer_add().
Changes since v2:
- Split the commit into two to separate transitively called function
updates from writer call-site adaptations
- Made formatting improvements in comments and code for better
readability.
- Modified the writer logic to flush and retry only when a specific
error occurs, while other errors are propagated as-is.
Meet Soni (2):
reftable: propagate specific error codes in block_writer_add()
reftable: adapt writer code to propagate block_writer_add() errors
reftable/block.c | 13 ++++++------
reftable/block.h | 2 +-
reftable/record.c | 53 +++++++++++++++++++++--------------------------
reftable/writer.c | 30 ++++++++++++++++-----------
4 files changed, 50 insertions(+), 48 deletions(-)
Range-diff against v2:
1: 7cdc7ce0ce ! 1: 6ab35d569c reftable: return proper error code from block_writer_add()
@@ Metadata
Author: Meet Soni [off-list ref]
## Commit message ##
- reftable: return proper error code from block_writer_add()
+ reftable: propagate specific error codes in block_writer_add()
- Previously, block_writer_add() used to return generic -1, which forced
- an assumption about the error type. Replace these generic -1 returns in
- block_writer_add() and related functions with defined error codes.
+ Previously, functions block_writer_add() and related functions returned
+ -1 when the record did not fit, forcing the caller to assume that any
+ failure meant the entry was too big. Replace these generic -1 returns
+ with defined error codes.
- Reviewed all call sites to ensure they check for nonzero error returns
- rather than strictly -1, confirming that this change is safe.
+ This prepares the codebase for finer-grained error handling so that
+ callers can distinguish between a block-full condition and other errors.
Signed-off-by: Meet Soni [off-list ref]
@@ reftable/block.c: uint8_t block_writer_type(struct block_writer *bw)
-/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on
- success. Returns REFTABLE_API_ERROR if attempting to write a record with
- empty key. */
-+/* Adds the reftable_record to the block. Returns 0 on success and
++/*
++ * Adds the reftable_record to the block. Returns 0 on success and
+ * appropriate error codes on failure.
+ */
int block_writer_add(struct block_writer *w, struct reftable_record *rec)
@@ reftable/block.h: int block_writer_init(struct block_writer *bw, uint8_t typ, ui
uint8_t block_writer_type(struct block_writer *bw);
-/* appends the record, or -1 if it doesn't fit. */
-+/* attempts to append the record. returns 0 on success or error code on failure. */
++/* Attempts to append the record. Returns 0 on success or error code on failure. */
int block_writer_add(struct block_writer *w, struct reftable_record *rec);
/* appends the key restarts, and compress the block if necessary. */
@@ reftable/record.c: static int reftable_log_record_encode(const void *rec, struct
string_view_consume(&s, n);
return start.len - s.len;
-
- ## reftable/writer.c ##
-@@ reftable/writer.c: static int writer_add_record(struct reftable_writer *w,
- goto done;
-
- /*
-- * Try to add the record to the writer again. If this still fails then
-- * the record does not fit into the block size.
-- *
-- * TODO: it would be great to have `block_writer_add()` return proper
-- * error codes so that we don't have to second-guess the failure
-- * mode here.
-+ * Try to add the record to the writer again.
- */
- err = block_writer_add(w->block_writer, rec);
-- if (err) {
-- err = REFTABLE_ENTRY_TOO_BIG_ERROR;
-+ if (err)
- goto done;
-- }
-
- done:
- return err;
-: ---------- > 2: a54d440dd3 reftable: adapt writer code to propagate block_writer_add() errors
--
2.34.1