qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 06/52] migration/rdma: Clean up two more harmless signed vs.


From: Markus Armbruster
Subject: Re: [PATCH 06/52] migration/rdma: Clean up two more harmless signed vs. unsigned issues
Date: Wed, 20 Sep 2023 15:11:18 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)

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!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]