reftable/blocksource: adjust `read_block()` to return `ssize_t`
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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt committed
Jan 20, 2025 at 17:17 UTC
7c4c1cbc0b94665d6a94ac7df385459346af5265
4 files changed
+31
-24
reftable/blocksource.c
+4
-4
@@ -24,8 +24,8 @@ static void reftable_buf_close(void *b UNUSED)
24
{
25
}
26
27
-static int reftable_buf_read_block(void *v, struct reftable_block *dest,
28
- uint64_t off, uint32_t size)
27
+static ssize_t reftable_buf_read_block(void *v, struct reftable_block *dest,
28
+ uint64_t off, uint32_t size)
29
{
30
struct reftable_buf *b = v;
31
assert(off + size <= b->len);
@@ -78,8 +78,8 @@ static void file_close(void *v)
78
reftable_free(b);
79
}
80
81
-static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
82
- uint32_t size)
81
+static ssize_t file_read_block(void *v, struct reftable_block *dest, uint64_t off,
82
+ uint32_t size)
83
{
84
struct file_block_source *b = v;
85
assert(off + size <= b->size);
reftable/reader.c
+17
-13
@@ -20,11 +20,11 @@ uint64_t block_source_size(struct reftable_block_source *source)
20
return source->ops->size(source->arg);
21
}
22
23
-int block_source_read_block(struct reftable_block_source *source,
24
- struct reftable_block *dest, uint64_t off,
25
- uint32_t size)
23
+ssize_t block_source_read_block(struct reftable_block_source *source,
24
+ struct reftable_block *dest, uint64_t off,
25
+ uint32_t size)
26
{
27
- int result = source->ops->read_block(source->arg, dest, off, size);
27
+ ssize_t result = source->ops->read_block(source->arg, dest, off, size);
28
dest->source = *source;
29
return result;
30
}
@@ -57,14 +57,17 @@ static int reader_get_block(struct reftable_reader *r,
57
struct reftable_block *dest, uint64_t off,
58
uint32_t sz)
59
{
60
+ ssize_t bytes_read;
61
if (off >= r->size)
62
return 0;
62
-
63
- if (off + sz > r->size) {
63
+ if (off + sz > r->size)
64
sz = r->size - off;
65
- }
65
67
- return block_source_read_block(&r->source, dest, off, sz);
66
+ bytes_read = block_source_read_block(&r->source, dest, off, sz);
67
+ if (bytes_read < 0)
68
+ return (int)bytes_read;
69
+
70
+ return 0;
71
}
72
73
enum reftable_hash reftable_reader_hash_id(struct reftable_reader *r)
@@ -601,6 +604,7 @@ int reftable_reader_new(struct reftable_reader **out,
604
struct reftable_reader *r;
605
uint64_t file_size = block_source_size(source);
606
uint32_t read_size;
607
+ ssize_t bytes_read;
608
int err;
609
610
REFTABLE_CALLOC_ARRAY(r, 1);
@@ -619,8 +623,8 @@ int reftable_reader_new(struct reftable_reader **out,
623
goto done;
624
}
625
622
- err = block_source_read_block(source, &header, 0, read_size);
623
- if (err != read_size) {
626
+ bytes_read = block_source_read_block(source, &header, 0, read_size);
627
+ if (bytes_read < 0 || (size_t)bytes_read != read_size) {
628
err = REFTABLE_IO_ERROR;
629
goto done;
630
}
@@ -645,9 +649,9 @@ int reftable_reader_new(struct reftable_reader **out,
649
r->hash_id = 0;
650
r->refcount = 1;
651
648
- err = block_source_read_block(source, &footer, r->size,
649
- footer_size(r->version));
650
- if (err != footer_size(r->version)) {
652
+ bytes_read = block_source_read_block(source, &footer, r->size,
653
+ footer_size(r->version));
654
+ if (bytes_read < 0 || (size_t)bytes_read != footer_size(r->version)) {
655
err = REFTABLE_IO_ERROR;
656
goto done;
657
}
reftable/reader.h
+3
-3
@@ -16,9 +16,9 @@ https://developers.google.com/open-source/licenses/bsd
16
17
uint64_t block_source_size(struct reftable_block_source *source);
18
19
-int block_source_read_block(struct reftable_block_source *source,
20
- struct reftable_block *dest, uint64_t off,
21
- uint32_t size);
19
+ssize_t block_source_read_block(struct reftable_block_source *source,
20
+ struct reftable_block *dest, uint64_t off,
21
+ uint32_t size);
22
void block_source_close(struct reftable_block_source *source);
23
24
/* metadata for a block type */
reftable/reftable-blocksource.h
+7
-4
@@ -31,10 +31,13 @@ struct reftable_block_source_vtable {
31
/* returns the size of a block source */
32
uint64_t (*size)(void *source);
33
34
- /* reads a segment from the block source. It is an error to read
35
- beyond the end of the block */
36
- int (*read_block)(void *source, struct reftable_block *dest,
37
- uint64_t off, uint32_t size);
34
+ /*
35
+ * Reads a segment from the block source. It is an error to read beyond
36
+ * the end of the block.
37
+ */
38
+ ssize_t (*read_block)(void *source, struct reftable_block *dest,
39
+ uint64_t off, uint32_t size);
40
+
41
/* mark the block as read; may return the data back to malloc */
42
void (*return_block)(void *source, struct reftable_block *blockp);
43