git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Meet Soni <meetsoni3017@gmail.com>
To: git@vger.kernel.org
Cc: Meet Soni <meetsoni3017@gmail.com>,
	Patrick Steinhardt <ps@pks.im>,
	Han-Wen Nienhuys <hanwen@google.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: [GSoC PATCH] reftable: return proper error code from block_writer_add()
Date: Thu,  6 Mar 2025 17:43:24 +0530	[thread overview]
Message-ID: <20250306121324.1315290-1-meetsoni3017@gmail.com> (raw)

Previously, block_writer_add() used to return generic -1, which forced
an assumption about the error type.

Replace generic -1 returns in block_writer_add() and related functions
with defined error codes.

Signed-off-by: Meet Soni <meetsoni3017@gmail.com>
---
This patch attempts to avoid making an assumption regarding error codes
returned by block_writer_add().
 reftable/block.c  |  9 +++++----
 reftable/record.c | 16 +++++++++++-----
 reftable/writer.c |  8 +-------
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/reftable/block.c b/reftable/block.c
index b14a8f1259..50fbac801a 100644
--- a/reftable/block.c
+++ b/reftable/block.c
@@ -49,7 +49,7 @@ static int block_writer_register_restart(struct block_writer *w, int n,
 	if (is_restart)
 		rlen++;
 	if (2 + 3 * rlen + n > w->block_size - w->next)
-		return -1;
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
 	if (is_restart) {
 		REFTABLE_ALLOC_GROW_OR_NULL(w->restarts, w->restart_len + 1,
 					    w->restart_cap);
@@ -115,8 +115,9 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 	int err;
 
 	err = reftable_record_key(rec, &w->scratch);
-	if (err < 0)
+	if (err < 0) {
 		goto done;
+	}
 
 	if (!w->scratch.len) {
 		err = REFTABLE_API_ERROR;
@@ -126,14 +127,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
 	n = reftable_encode_key(&is_restart, out, last, w->scratch,
 				reftable_record_val_type(rec));
 	if (n < 0) {
-		err = -1;
+		err = n;
 		goto done;
 	}
 	string_view_consume(&out, n);
 
 	n = reftable_record_encode(rec, out, w->hash_size);
 	if (n < 0) {
-		err = -1;
+		err = n;
 		goto done;
 	}
 	string_view_consume(&out, n);
diff --git a/reftable/record.c b/reftable/record.c
index 8919df8a4d..5523804a0c 100644
--- a/reftable/record.c
+++ b/reftable/record.c
@@ -148,18 +148,18 @@ int reftable_encode_key(int *restart, struct string_view dest,
 	uint64_t suffix_len = key.len - prefix_len;
 	int n = put_var_int(&dest, prefix_len);
 	if (n < 0)
-		return -1;
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
 	string_view_consume(&dest, n);
 
 	*restart = (prefix_len == 0);
 
 	n = put_var_int(&dest, suffix_len << 3 | (uint64_t)extra);
 	if (n < 0)
-		return -1;
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
 	string_view_consume(&dest, n);
 
 	if (dest.len < suffix_len)
-		return -1;
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
 	memcpy(dest.buf, key.buf + prefix_len, suffix_len);
 	string_view_consume(&dest, suffix_len);
 
@@ -1144,14 +1144,20 @@ static struct reftable_record_vtable reftable_index_record_vtable = {
 
 int reftable_record_key(struct reftable_record *rec, struct reftable_buf *dest)
 {
-	return reftable_record_vtable(rec)->key(reftable_record_data(rec), dest);
+	int key_len = reftable_record_vtable(rec)->key(reftable_record_data(rec), dest);
+	if (key_len < 0)
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
+	return key_len;
 }
 
 int reftable_record_encode(struct reftable_record *rec, struct string_view dest,
 			   uint32_t hash_size)
 {
-	return reftable_record_vtable(rec)->encode(reftable_record_data(rec),
+	int encode_len = reftable_record_vtable(rec)->encode(reftable_record_data(rec),
 						   dest, hash_size);
+	if (encode_len < 0)
+		return REFTABLE_ENTRY_TOO_BIG_ERROR;
+	return encode_len;
 }
 
 int reftable_record_copy_from(struct reftable_record *rec,
diff --git a/reftable/writer.c b/reftable/writer.c
index f3ab1035d6..600ba5441b 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -327,16 +327,10 @@ 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;
 		goto done;
 	}
 

base-commit: e969bc875963a10890d61ba84eab3a460bd9e535
-- 
2.34.1


             reply	other threads:[~2025-03-06 12:13 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-06 12:13 Meet Soni [this message]
2025-03-06 14:43 ` [GSoC PATCH] reftable: return proper error code from block_writer_add() Patrick Steinhardt
2025-03-06 17:04 ` Junio C Hamano
2025-03-08 13:33 ` [GSoC PATCH v2] " Meet Soni
2025-03-12  8:23   ` Patrick Steinhardt
2025-03-12 12:11   ` [GSoC PATCH v3 0/2] reftable: return proper error codes from block_writer_add Meet Soni
2025-03-12 12:11     ` [PATCH v3 1/2] reftable: propagate specific error codes in block_writer_add() Meet Soni
2025-03-12 12:11     ` [PATCH v3 2/2] reftable: adapt writer code to propagate block_writer_add() errors Meet Soni
2025-03-12 12:49       ` Patrick Steinhardt
2025-03-13 15:29         ` Meet Soni
2025-03-19 13:18           ` Patrick Steinhardt
2025-03-19  7:59     ` [GSoC PATCH v4 0/3] reftable: return proper error codes from block_writer_add Meet Soni
2025-03-19  7:59       ` [PATCH v4 1/3] reftable: propagate specific error codes in block_writer_add() Meet Soni
2025-03-19  7:59       ` [PATCH v4 2/3] reftable: adapt writer_add_record() to propagate block_writer_add() errors Meet Soni
2025-03-19  7:59       ` [PATCH v4 3/3] reftable: adapt write_object_record() " Meet Soni
2025-03-19 15:29       ` [GSoC PATCH v5 0/3] reftable: return proper error codes from block_writer_add Meet Soni
2025-03-19 15:29         ` [GSoC PATCH v5 1/3] reftable: propagate specific error codes in block_writer_add() Meet Soni
2025-03-19 15:29         ` [GSoC PATCH v5 2/3] reftable: adapt writer_add_record() to propagate block_writer_add() errors Meet Soni
2025-03-19 15:29         ` [GSoC PATCH v5 3/3] reftable: adapt write_object_record() " Meet Soni
2025-03-19 15:48         ` [GSoC PATCH v5 0/3] reftable: return proper error codes from block_writer_add Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250306121324.1315290-1-meetsoni3017@gmail.com \
    --to=meetsoni3017@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hanwen@google.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).