From: Marcel Apfelbaum <marcel@redhat.com>
To: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: yuval.shaia@oracle.com, mst@redhat.com
Subject: Re: [Qemu-devel] [PATCH for-2.12] hw/rdma: fix clang compilation errors
Date: Wed, 21 Mar 2018 15:23:00 +0200 [thread overview]
Message-ID: <1fa66dae-b6e4-af3c-f3ec-8e4dd03d74f0@redhat.com> (raw)
In-Reply-To: <8fc94472-c9dc-edf6-d89b-90f6d6ac281a@redhat.com>
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
prev parent reply other threads:[~2018-03-21 13:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1fa66dae-b6e4-af3c-f3ec-8e4dd03d74f0@redhat.com \
--to=marcel@redhat.com \
--cc=eblake@redhat.com \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=yuval.shaia@oracle.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).