[GSoC,v3,0/2] reftable: return proper error codes from block_writer_add

Message

Meet Soni March 12, 2025, 12:11 p.m. UTC

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 <[e-mail]>
     
      ## 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 <[e-mail]>
     
    @@ 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