I couldn't find anything quickly in the realloc() manpage about how it handles a size of zero, that's why I added it. I now checked the POSIX documents and they clearly state that size of zero with ptr being non-NULL will mean ptr is freed, so I don't need to check total before reallocf().

But POSIX also states that in that case it may return NULL or any other valid pointer that may be passed to free(). I guess that means I can refactor this somewhat, but I can't assume realloc() to return NULL when a zero size is passed. (I generally assume reallocf() will work exactly the same.)

The added check only serves to confuse the issue. The outcome will be exactly the same.

As I mentioned in the comment above, POSIX says that it's up to the implementation whether realloc() returns NULL when given a zero size. It may also return a pointer which can be freed. I'd prefer not to make an assumption on this implementation detail here.

I know. It makes no difference.

Sure, but it's still clearer to check for this case explicitly.

Clarity is in the eye of the beholder.

Right now you have the following possibilities:

• total > 0, allocation succeeds: iov_to_buf() successful, returns non-zero.
• total > 0, allocation fails: *buf is clobbered, original buffer is freed, iov_to_buf() returns zero.
• total == 0: *buf is clobbered with either NULL or a non-NULL pointer to a zero-length object, original buffer is freed, iov_to_buf() returns zero.

I don't find that clear at all.

In fact I think this entire change is a bad idea. The two functions are fine as they are. You are giving up the ability to tell an error apart from a zero-length iov for the sake of avoiding a single cast.

The last two cases are actually the same: zero is returned, the original buffer is freed, and *buf contains a pointer that can be passed to free(). There exactly two code paths that use of this function, both of them in pci_virtio_scsi.c. One of them doesn't even check the returned value, and it would break in interesting ways if it was -1, but it does the right thing in case of 0. The other is the assertion you've seen in the build error.

That being said, being fast and loose with signedness on a value that is naturally unsigned just to be able indicate an error by returning -1, while at the same time instructing the compiler to error out on mismatched signedness comparisons, and then papering over it with an explicit cast isn't really all that great either, especially not when the error case returning -1 isn't all that interesting or necessary to begin with.

This is due to malloc(0) being special.

However, my point is that the rest of the function (including the for loop below that iterates over the iovec) will handle a size of zero just fine. There's no need to handle a size of 0 as a special case.