| Message ID | 20230918144206.560120-7-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/rdma: Error handling fixes | expand |
Markus Armbruster <armbru@redhat.com> writes: > qemu_rdma_exchange_get_response() compares int parameter @expecting > with uint32_t head->type. Actual arguments are non-negative > enumeration constants, RDMAControlHeader uint32_t member type, or > qemu_rdma_exchange_recv() int parameter expecting. Actual arguments > for the latter are non-negative enumeration constants. Change both > parameters to uint32_t. > > In qio_channel_rdma_readv(), loop control variable @i is ssize_t, and > counts from 0 up to @niov, which is size_t. Change @i to size_t. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> just a comment... > static int qemu_rdma_exchange_recv(RDMAContext *rdma, RDMAControlHeader *head, > - int expecting) > + uint32_t expecting) > { > RDMAControlHeader ready = { > .len = 0, > @@ -2851,7 +2851,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, > RDMAContext *rdma; > RDMAControlHeader head; > int ret = 0; > - ssize_t i; > + size_t i; > size_t done = 0; It seems the idea was for 'done' to be ssize_t like in qio_channel_rdma_writev()
Fabiano Rosas <farosas@suse.de> writes: > Markus Armbruster <armbru@redhat.com> writes: > >> qemu_rdma_exchange_get_response() compares int parameter @expecting >> with uint32_t head->type. Actual arguments are non-negative >> enumeration constants, RDMAControlHeader uint32_t member type, or >> qemu_rdma_exchange_recv() int parameter expecting. Actual arguments >> for the latter are non-negative enumeration constants. Change both >> parameters to uint32_t. >> >> In qio_channel_rdma_readv(), loop control variable @i is ssize_t, and >> counts from 0 up to @niov, which is size_t. Change @i to size_t. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > Reviewed-by: Fabiano Rosas <farosas@suse.de> > > just a comment... > >> static int qemu_rdma_exchange_recv(RDMAContext *rdma, RDMAControlHeader *head, >> - int expecting) >> + uint32_t expecting) >> { >> RDMAControlHeader ready = { >> .len = 0, >> @@ -2851,7 +2851,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, >> RDMAContext *rdma; >> RDMAControlHeader head; >> int ret = 0; >> - ssize_t i; >> + size_t i; >> size_t done = 0; > > It seems the idea was for 'done' to be ssize_t like in > qio_channel_rdma_writev() You're right, the two functions are still inconsistent: @done is size_t here and ssize_t there. Hmm, there's yet another mess: ret = qemu_rdma_fill(rdma, data, want, 0); done += ret; want -= ret; qemu_rdma_fill() returns size_t, @done is size_t or ssize_t, @want is @size_t, but @ret is int. Unwanted truncation is theoretically possible. Separate patch. Thanks!
diff --git a/migration/rdma.c b/migration/rdma.c index 4328610a4c..e3b8d0506c 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -1801,7 +1801,7 @@ static int qemu_rdma_post_recv_control(RDMAContext *rdma, int idx) * Block and wait for a RECV control channel message to arrive. */ static int qemu_rdma_exchange_get_response(RDMAContext *rdma, - RDMAControlHeader *head, int expecting, int idx) + RDMAControlHeader *head, uint32_t expecting, int idx) { uint32_t byte_len; int ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RECV_CONTROL + idx, @@ -1959,7 +1959,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head, * control-channel message. */ static int qemu_rdma_exchange_recv(RDMAContext *rdma, RDMAControlHeader *head, - int expecting) + uint32_t expecting) { RDMAControlHeader ready = { .len = 0, @@ -2851,7 +2851,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc, RDMAContext *rdma; RDMAControlHeader head; int ret = 0; - ssize_t i; + size_t i; size_t done = 0; RCU_READ_LOCK_GUARD();
qemu_rdma_exchange_get_response() compares int parameter @expecting with uint32_t head->type. Actual arguments are non-negative enumeration constants, RDMAControlHeader uint32_t member type, or qemu_rdma_exchange_recv() int parameter expecting. Actual arguments for the latter are non-negative enumeration constants. Change both parameters to uint32_t. In qio_channel_rdma_readv(), loop control variable @i is ssize_t, and counts from 0 up to @niov, which is size_t. Change @i to size_t. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- migration/rdma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)