diff mbox series

[v2,09/10] reftable/blocksource: adjust `read_block()` to return `ssize_t`

Message ID 20250120-b4-pks-reftable-sign-compare-v2-9-b4566d02e4a5@pks.im (mailing list archive)
State New
Headers show
Series reftable: fix -Wsign-compare warnings | expand

Commit Message

Patrick Steinhardt Jan. 20, 2025, 4:17 p.m. UTC
The `block_source_read_block()` function and its implementations return
an integer as a result that reflects either the number of bytes read, or
an error. As such its return type, a signed integer, isn't wrong, but it
doesn't give the reader a good hint what it actually returns.

Refactor the function to return an `ssize_t` instead, which is typical
for functions similar to read(3p) and should thus give readers a better
signal what they can expect as a result.

Adjust callers to better handle the returned value to avoid warnings
with -Wsign-compare. One of these callers is `reader_get_block()`, whose
return value is only ever used by its callers to figure out whether or
not the read was successful. So instead of bubbling up the `ssize_t`
there, too, we adapt it to only indicate success or errors.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 reftable/blocksource.c          |  8 ++++----
 reftable/reader.c               | 30 +++++++++++++++++-------------
 reftable/reader.h               |  6 +++---
 reftable/reftable-blocksource.h | 11 +++++++----
 4 files changed, 31 insertions(+), 24 deletions(-)
diff mbox series

Patch

diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 52e0915a67..bba4a45b98 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -24,8 +24,8 @@  static void reftable_buf_close(void *b UNUSED)
 {
 }
 
-static int reftable_buf_read_block(void *v, struct reftable_block *dest,
-				   uint64_t off, uint32_t size)
+static ssize_t reftable_buf_read_block(void *v, struct reftable_block *dest,
+				       uint64_t off, uint32_t size)
 {
 	struct reftable_buf *b = v;
 	assert(off + size <= b->len);
@@ -78,8 +78,8 @@  static void file_close(void *v)
 	reftable_free(b);
 }
 
-static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
-			   uint32_t size)
+static ssize_t file_read_block(void *v, struct reftable_block *dest, uint64_t off,
+			       uint32_t size)
 {
 	struct file_block_source *b = v;
 	assert(off + size <= b->size);
diff --git a/reftable/reader.c b/reftable/reader.c
index 9df8a5ecb1..3f2e4b2800 100644
--- a/reftable/reader.c
+++ b/reftable/reader.c
@@ -20,11 +20,11 @@  uint64_t block_source_size(struct reftable_block_source *source)
 	return source->ops->size(source->arg);
 }
 
-int block_source_read_block(struct reftable_block_source *source,
-			    struct reftable_block *dest, uint64_t off,
-			    uint32_t size)
+ssize_t block_source_read_block(struct reftable_block_source *source,
+				struct reftable_block *dest, uint64_t off,
+				uint32_t size)
 {
-	int result = source->ops->read_block(source->arg, dest, off, size);
+	ssize_t result = source->ops->read_block(source->arg, dest, off, size);
 	dest->source = *source;
 	return result;
 }
@@ -57,14 +57,17 @@  static int reader_get_block(struct reftable_reader *r,
 			    struct reftable_block *dest, uint64_t off,
 			    uint32_t sz)
 {
+	ssize_t bytes_read;
 	if (off >= r->size)
 		return 0;
-
-	if (off + sz > r->size) {
+	if (off + sz > r->size)
 		sz = r->size - off;
-	}
 
-	return block_source_read_block(&r->source, dest, off, sz);
+	bytes_read = block_source_read_block(&r->source, dest, off, sz);
+	if (bytes_read < 0)
+		return (int)bytes_read;
+
+	return 0;
 }
 
 enum reftable_hash reftable_reader_hash_id(struct reftable_reader *r)
@@ -601,6 +604,7 @@  int reftable_reader_new(struct reftable_reader **out,
 	struct reftable_reader *r;
 	uint64_t file_size = block_source_size(source);
 	uint32_t read_size;
+	ssize_t bytes_read;
 	int err;
 
 	REFTABLE_CALLOC_ARRAY(r, 1);
@@ -619,8 +623,8 @@  int reftable_reader_new(struct reftable_reader **out,
 		goto done;
 	}
 
-	err = block_source_read_block(source, &header, 0, read_size);
-	if (err != read_size) {
+	bytes_read = block_source_read_block(source, &header, 0, read_size);
+	if (bytes_read < 0 || (size_t)bytes_read != read_size) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
@@ -645,9 +649,9 @@  int reftable_reader_new(struct reftable_reader **out,
 	r->hash_id = 0;
 	r->refcount = 1;
 
-	err = block_source_read_block(source, &footer, r->size,
-				      footer_size(r->version));
-	if (err != footer_size(r->version)) {
+	bytes_read = block_source_read_block(source, &footer, r->size,
+					     footer_size(r->version));
+	if (bytes_read < 0 || (size_t)bytes_read != footer_size(r->version)) {
 		err = REFTABLE_IO_ERROR;
 		goto done;
 	}
diff --git a/reftable/reader.h b/reftable/reader.h
index d2b48a4849..bb72108a6f 100644
--- a/reftable/reader.h
+++ b/reftable/reader.h
@@ -16,9 +16,9 @@  license that can be found in the LICENSE file or at
 
 uint64_t block_source_size(struct reftable_block_source *source);
 
-int block_source_read_block(struct reftable_block_source *source,
-			    struct reftable_block *dest, uint64_t off,
-			    uint32_t size);
+ssize_t block_source_read_block(struct reftable_block_source *source,
+				struct reftable_block *dest, uint64_t off,
+				uint32_t size);
 void block_source_close(struct reftable_block_source *source);
 
 /* metadata for a block type */
diff --git a/reftable/reftable-blocksource.h b/reftable/reftable-blocksource.h
index f06ad52e0a..6b326aa5ea 100644
--- a/reftable/reftable-blocksource.h
+++ b/reftable/reftable-blocksource.h
@@ -31,10 +31,13 @@  struct reftable_block_source_vtable {
 	/* returns the size of a block source */
 	uint64_t (*size)(void *source);
 
-	/* reads a segment from the block source. It is an error to read
-	   beyond the end of the block */
-	int (*read_block)(void *source, struct reftable_block *dest,
-			  uint64_t off, uint32_t size);
+	/*
+	 * Reads a segment from the block source. It is an error to read beyond
+	 * the end of the block.
+	 */
+	ssize_t (*read_block)(void *source, struct reftable_block *dest,
+			     uint64_t off, uint32_t size);
+
 	/* mark the block as read; may return the data back to malloc */
 	void (*return_block)(void *source, struct reftable_block *blockp);