On 16.06.21 15:18, Paolo Bonzini wrote:
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?
The reason it’s different is that the comment style in question was
added to checkpatch only relatively recently. I can’t speak for others,
but I’m a simple person. I just do what makes checkpatch happy. :)
Given that the checkpatch complaint is only a warning, I think it’s OK
to keep the comment as it is here, and perhaps optionally fix all
comments in block_int.h in a follow-up. I don’t think we need to fix
existing comments, but, well, it wouldn’t be wrong.
Max
