hw/rdma: fix clang compilation errors
PATCH 1/1 hw/rdma: fix clang compilation errors
Fix some enum castings and extra parentheses.
--- hw/rdma/vmw/pvrdma_cmd.c | 5 +++-- hw/rdma/vmw/pvrdma_main.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)
hw/rdma/vmw/pvrdma_cmd.c +3 -2
@@ -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 ))) { tbl_idx =0 ; dir_idx++;pr_dbg ("Mapping to table %d\n" , dir_idx);
@@ -507,7 +507,8 @@ static int modify_qp(PVRDMADev *dev, union pvrdma_cmd_req *req, rsp->hdr.err =rdma_rm_modify_qp (&dev->rdma_dev_res, &dev->backend_dev, cmd->qp_handle, cmd->attr_mask, (union ibv_gid *)&cmd->attrs.ah_attr.grh.dgid, - cmd->attrs.dest_qp_num, cmd->attrs.qp_state, + (enum ibv_qp_state )cmd->attrs.dest_qp_num, + (enum ibv_qp_state )cmd->attrs.qp_state, cmd->attrs.qkey, cmd->attrs.rq_psn, cmd->attrs.sq_psn);
hw/rdma/vmw/pvrdma_main.c +3 -2
@@ -91,7 +91,7 @@ static int init_dev_ring(PvrdmaRing *ring, struct pvrdma_ring **ring_state, goto out_free_tbl; }/* RX ring is the second */ - (struct pvrdma_ring *)(*ring_state)++; + (*ring_state)++; rc = pvrdma_ring_init(ring, name, pci_dev, (struct pvrdma_ring *)*ring_state, (num_pages - 1) * TARGET_PAGE_SIZE /
@@ -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 ; dev->rdma_dev_res.ports[i].pkey_tbl = g_malloc0(sizeof(*dev->rdma_dev_res.ports[i].pkey_tbl) *
Signature · 1 line
2.13.5
Quoted 23 lines · Marcel Apfelbaum
Fix some enum castings and extra parentheses.
Reported-by Michael S. Tsirkin <[email protected]>Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/rdma/vmw/pvrdma_cmd.c | 5 +++-- hw/rdma/vmw/pvrdma_main.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)hw/rdma/vmw/pvrdma_cmd.c +1
-1@@ -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 ))) { tbl_idx =0 ; dir_idx++;pr_dbg ("Mapping to table %d\n" , dir_idx);
hw/rdma/vmw/pvrdma_cmd.c static int modify_qp(PVRDMADev *dev, union pvrdma_cmd_req *req, · line 510
Quoted 5 lines
hw/rdma/vmw/pvrdma_cmd.c +1
-1@@ -507,7 +507,8 @@ static int modify_qp(PVRDMADev *dev, union pvrdma_cmd_req *req, rsp->hdr.err = rdma_rm_modify_qp(&dev->rdma_dev_res, &dev->backend_dev, cmd->qp_handle, cmd->attr_mask, (union ibv_gid *)&cmd->attrs.ah_attr.grh.dgid, - cmd->attrs.dest_qp_num, cmd->attrs.qp_state, + (enum ibv_qp_state)cmd->attrs.dest_qp_num,
This field is uint32_t and function gets uint32_t so we should be fine, it doesn't make sense to cast it to ibv_qp_state.
Quoted 18 lines
hw/rdma/vmw/pvrdma_cmd.c +1
-0+ (enum ibv_qp_state )cmd->attrs.qp_state, cmd->attrs.qkey, cmd->attrs.rq_psn, cmd->attrs.sq_psn);hw/rdma/vmw/pvrdma_main.c +1
-1@@ -91,7 +91,7 @@ static int init_dev_ring(PvrdmaRing *ring, struct pvrdma_ring **ring_state, goto out_free_tbl; }/* RX ring is the second */ - (struct pvrdma_ring *)(*ring_state)++; + (*ring_state)++; rc = pvrdma_ring_init(ring, name, pci_dev, (struct pvrdma_ring *)*ring_state, (num_pages - 1) * TARGET_PAGE_SIZE /
hw/rdma/vmw/pvrdma_main.c static void init_ports(PVRDMADev *dev, Error **errp) · line 296
Quoted 6 lines
hw/rdma/vmw/pvrdma_main.c +2
-1@@ -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 ;
Suggesting to set it to IBV_PORT_DOWN and avoid this casting.
Quoted 6 lines
hw/rdma/vmw/pvrdma_main.c +0
-0dev->rdma_dev_res.ports[i].pkey_tbl = g_malloc0(sizeof(*dev->rdma_dev_res.ports[i].pkey_tbl) *Signature · 1 line
2.13.5
Quoted 14 lines · Marcel Apfelbaum
Fix some enum castings and extra parentheses.
Reported-by Michael S. Tsirkin <[email protected]>Signed-off-by Marcel Apfelbaum <[email protected]>--- hw/rdma/vmw/pvrdma_cmd.c | 5 +++-- hw/rdma/vmw/pvrdma_main.c | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-)hw/rdma/vmw/pvrdma_cmd.c +0
-0
hw/rdma/vmw/pvrdma_cmd.c static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma, · line 76
Quoted 5 lines
hw/rdma/vmw/pvrdma_cmd.c +1
-1@@ -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)) {
hw/rdma/vmw/pvrdma_cmd.c static void init_ports(PVRDMADev *dev, Error **errp) · line 297
Quoted 8 lines
hw/rdma/vmw/pvrdma_cmd.c +2
-1@@ -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).
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 23 lines · Eric Blake
Quoted 14 lines · Marcel Apfelbaum
Fix some enum castings and extra parentheses.
Reported-by Michael S. Tsirkin <[email protected]>Signed-off-by Marcel Apfelbaum <[email protected]>---
hw/rdma/vmw/pvrdma_cmd.c | 5 +++--
hw/rdma/vmw/pvrdma_main.c| 5 +++--2 files changed, 6 insertions(+), 4 deletions(-)hw/rdma/vmw/pvrdma_cmd.c +0
-0hw/rdma/vmw/pvrdma_cmd.c static void *pvrdma_map_to_pdir(PCIDevice *pdev, uint64_t pdir_dma, · line 76
Quoted 5 lines
hw/rdma/vmw/pvrdma_cmd.c +1
-1@@ -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
Quoted 2 lines
if (
tbl_idx==TARGET_PAGE_SIZE/sizeof(uint64_t)) {
I'll try, thanks.
Quoted 12 lines
Quoted 7 lines
hw/rdma/vmw/pvrdma_cmd.c +2
-1@@ -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_DOWNinstead of having to cast? (Even ifIBV_PORT_DOWNandPVRDMA_PORT_DOWNboth 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