* [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
@ 2018-03-21 12:40 Marcel Apfelbaum
2018-03-21 12:56 ` Yuval Shaia
2018-03-21 13:20 ` Eric Blake
0 siblings, 2 replies; 4+ messages in thread
From: Marcel Apfelbaum @ 2018-03-21 12:40 UTC (permalink / raw)
To: qemu-devel; +Cc: yuval.shaia, marcel, mst
Fix some enum castings and extra parentheses.
Reported-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
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))) {
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);
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 99787812ba..0e3469287f 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -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) *
--
2.13.5
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
2018-03-21 12:40 [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors Marcel Apfelbaum
@ 2018-03-21 12:56 ` Yuval Shaia
2018-03-21 13:20 ` Eric Blake
1 sibling, 0 replies; 4+ messages in thread
From: Yuval Shaia @ 2018-03-21 12:56 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, mst, yuval.shaia
On Wed, Mar 21, 2018 at 02:40:26PM +0200, Marcel Apfelbaum wrote:
> Fix some enum castings and extra parentheses.
>
> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> 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))) {
> 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,
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.
> + (enum ibv_qp_state)cmd->attrs.qp_state,
> cmd->attrs.qkey, cmd->attrs.rq_psn,
> cmd->attrs.sq_psn);
>
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index 99787812ba..0e3469287f 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -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;
Suggesting to set it to IBV_PORT_DOWN and avoid this casting.
>
> dev->rdma_dev_res.ports[i].pkey_tbl =
> g_malloc0(sizeof(*dev->rdma_dev_res.ports[i].pkey_tbl) *
> --
> 2.13.5
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
2018-03-21 12:40 [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors Marcel Apfelbaum
2018-03-21 12:56 ` Yuval Shaia
@ 2018-03-21 13:20 ` Eric Blake
2018-03-21 13:23 ` Marcel Apfelbaum
1 sibling, 1 reply; 4+ messages in thread
From: Eric Blake @ 2018-03-21 13:20 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: yuval.shaia, mst
On 03/21/2018 07:40 AM, Marcel Apfelbaum wrote:
> Fix some enum castings and extra parentheses.
>
> Reported-by: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
> 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)) {
> @@ -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).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
2018-03-21 13:20 ` Eric Blake
@ 2018-03-21 13:23 ` Marcel Apfelbaum
0 siblings, 0 replies; 4+ messages in thread
From: Marcel Apfelbaum @ 2018-03-21 13:23 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: yuval.shaia, mst
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 <mst@redhat.com>
>> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-21 13:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-21 12:40 [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors Marcel Apfelbaum
2018-03-21 12:56 ` Yuval Shaia
2018-03-21 13:20 ` Eric Blake
2018-03-21 13:23 ` Marcel Apfelbaum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).