Subject: Re: [PATCH v6 04/20] reftable: utility functions

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 13 Apr 2021 14:56:40 +0200

On Tue, Apr 13 2021, Han-Wen Nienhuys wrote:

> On Tue, Apr 13, 2021 at 10:02 AM Ævar Arnfjörð Bjarmason
> <[email-masked]> wrote:
>>
>>
>> On Mon, Apr 12 2021, Han-Wen Nienhuys via GitGitGadget wrote:
>>
>> > +int strbuf_add_void(void *b, const void *data, size_t sz)
>> > +{
>> > +     strbuf_add((struct strbuf *)b, data, sz);
>> > +     return sz;
>> > +}
>>
>> Is that cast needed on your compiler? This compiles without warnings for
>> me without that.
>
> No! thanks.

Some more, a combination of redundant casting and casting early to
another variable being easier to read. The latter being a matter of
style/opinion. Some more like that in the code:

    git grep -W '\(\(struct .*\*'

diff:

diff --git a/reftable/blocksource.c b/reftable/blocksource.c
index 25d4d95b52b..91eb0b495f5 100644
--- a/reftable/blocksource.c
+++ b/reftable/blocksource.c
@@ -26,7 +26,7 @@ static void strbuf_close(void *b)
 static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,
 			     uint32_t size)
 {
-	struct strbuf *b = (struct strbuf *)v;
+	struct strbuf *b = v;
 	assert(off + size <= b->len);
 	dest->data = reftable_calloc(size);
 	memcpy(dest->data, b->buf + off, size);
@@ -36,7 +36,8 @@ static int strbuf_read_block(void *v, struct reftable_block *dest, uint64_t off,
 
 static uint64_t strbuf_size(void *b)
 {
-	return ((struct strbuf *)b)->len;
+	struct strbuf *buf = b;
+	return buf->len;
 }
 
 static struct reftable_block_source_vtable strbuf_vtable = {
@@ -91,10 +92,11 @@ static void file_return_block(void *b, struct reftable_block *dest)
 
 static void file_close(void *b)
 {
-	int fd = ((struct file_block_source *)b)->fd;
+	struct file_block_source *bs =  b;
+	int fd = bs->fd;
 	if (fd > 0) {
 		close(fd);
-		((struct file_block_source *)b)->fd = 0;
+		bs->fd = 0;
 	}
 
 	reftable_free(b);
@@ -103,7 +105,7 @@ static void file_close(void *b)
 static int file_read_block(void *v, struct reftable_block *dest, uint64_t off,
 			   uint32_t size)
 {
-	struct file_block_source *b = (struct file_block_source *)v;
+	struct file_block_source *b = v;
 	assert(off + size <= b->size);
 	dest->data = reftable_malloc(size);
 	if (pread(b->fd, dest->data, size, off) != size)