qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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

      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).