On 15/06/21 18:18, Max Reitz wrote:
}
+/* Returns the maximum hardware transfer length, in bytes; guaranteed
nonzero */
+uint64_t blk_get_max_hw_transfer(BlockBackend *blk)
+{
+ BlockDriverState *bs = blk_bs(blk);
+ uint64_t max = INT_MAX;
+
+ if (bs) {
+ max = MIN_NON_ZERO(bs->bl.max_hw_transfer, bs->bl.max_transfer);
+ }
+ return max;
Both `max_hw_transfer` and `max_transfer` can be 0, so this could return
0, contrary to what the comment above promises.
Should `max` be initialized to 0 with a `MIN_NON_ZERO(max, INT_MAX)`
here (like `blk_get_max_transfer()` does it)?
Yes, something to that effect.
(As for the rest, I think aligning to the request alignment makes sense,
but then again we don’t do that for max_transfer either, so... this at
least wouldn’t be a new bug.
Ok, will do. I will also add a new patch to align max_transfer to the
request alignment.
Regarding the comment, checkpatch complains about it, so it should be
fixed so that /* is on its own line.
That makes it different from every other comment in block_int.h though.
Is it okay to fix all of them in a follow-up?
Paolo
Speaking of checkpatch, now that I ran it, it also complains about the
new line in bdrv_merge_limits() exceeding 80 characters, so that should
be fixed, too.)