qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors


From: Marcel Apfelbaum
Subject: Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
Date: Wed, 21 Mar 2018 15:23:00 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:52.0) Gecko/20100101 Thunderbird/52.6.0

On 21/03/2018 15:20, Eric Blake wrote:
> On 03/21/2018 07:40 AM, Marcel Apfelbaum wrote:
>> Fix some enum castings and extra parentheses.
>>
>> Reported-by: Michael S. Tsirkin <address@hidden>
>> Signed-off-by: Marcel Apfelbaum <address@hidden>
>> ---
>>   hw/rdma/vmw/pvrdma_cmd.c  | 5 +++--
>>   hw/rdma/vmw/pvrdma_main.c | 5 +++--
>>   2 files changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/rdma/vmw/pvrdma_cmd.c b/hw/rdma/vmw/pvrdma_cmd.c
>> index 293dfed29f..25f747a190 100644
>> --- a/hw/rdma/vmw/pvrdma_cmd.c
>> +++ b/hw/rdma/vmw/pvrdma_cmd.c
>> @@ -73,7 +73,7 @@ static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t 
>> pdir_dma,
>>       tbl_idx = 1;
>>       addr_idx = 1;
>>       while (addr_idx < nchunks) {
>> -        if ((tbl_idx == (TARGET_PAGE_SIZE / sizeof(uint64_t)))) {
>> +        if (tbl_idx == (TARGET_PAGE_SIZE / sizeof(uint64_t))) {
> 
> Can't you still simplify that further to
> 
> if (tbl_idx == TARGET_PAGE_SIZE / sizeof(uint64_t)) {
> 

I'll try, thanks.

>> @@ -292,7 +292,8 @@ static void init_ports(PVRDMADev *dev, Error **errp)
>>       memset(dev->rdma_dev_res.ports, 0, sizeof(dev->rdma_dev_res.ports));
>>         for (i = 0; i < MAX_PORTS; i++) {
>> -        dev->rdma_dev_res.ports[i].state = PVRDMA_PORT_DOWN;
>> +        dev->rdma_dev_res.ports[i].state =
>> +            (enum ibv_port_state)PVRDMA_PORT_DOWN;
>>   
> 
> This one looks suspicious - shouldn't you instead be using IBV_PORT_DOWN 
> instead of having to cast?  (Even if
> IBV_PORT_DOWN and PVRDMA_PORT_DOWN both have the value of 1 for now, the 
> compiler warning is telling you that either one
> of the two enums can change independently in the future, and using a cast to 
> shut up the compiler feels unsafe).
> 

Yes, Yuval pointed it out too, I'll change it.

Thanks,
Marcel



reply via email to

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