On 09.06.22 17:27, Alberto Faria wrote:
For consistency with other I/O functions, and in preparation to implement bdrv_{pread,pwrite}() using generated_co_wrapper.unsigned int fits in int64_t, so all callers remain correct. bdrv_check_request32() is called further down the stack and causes -EIO to be returned if 'bytes' is negative or greater than BDRV_REQUEST_MAX_BYTES, which in turns never exceeds SIZE_MAX.
I’m not a huge fan of that reasoning alone. I don’t like generating an object that will be invalid if `bytes > SIZE_MAX`, and then rely on some later check in a different context verifying that `bytes <= SIZE_MAX`. In theory, if the latter check is removed, we might forget caring for the former. (In practice, such a case (where I/O vectors remain using size_t, but we allow for larger overall requests) is difficult to imagine, though.)
However, bdrv_check_request32() also calls bdrv_check_qiov_request(), which verifies the integrity of qiov by checking that `bytes` will not exceed `qiov->size - qiov_offset`. So if we had any overflow when casting `bytes` to `size_t`, it’ll be seen there directly (and I don’t see why we’d remove that specific check).
Given that, and given that there’s precedent (e.g. bdrv_pread()), I’m OK with the change.
Reviewed-by: Hanna Reitz <hre...@redhat.com>
