[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