diff mbox series

[v3,02/12] block/iscsi:Remove redundant statement in iscsi_open()

Message ID 20200302130715.29440-4-kuhn.chenqun@huawei.com
State New
Headers show
Series redundant code: Fix warnings reported by Clang static code analyzer | expand

Commit Message

Chen Qun March 2, 2020, 1:07 p.m. UTC
Clang static code analyzer show warning:
  block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read
        flags &= ~BDRV_O_RDWR;
        ^        ~~~~~~~~~~~~

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
---
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Lieven <pl@kamp.de>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>

v1->v2:
 Keep the 'flags' then use it(Base on Kevin's comments).
---
 block/iscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kevin Wolf March 10, 2020, 2:26 p.m. UTC | #1
Am 02.03.2020 um 14:07 hat Chen Qun geschrieben:
> Clang static code analyzer show warning:
>   block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read
>         flags &= ~BDRV_O_RDWR;
>         ^        ~~~~~~~~~~~~
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
> ---
> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Lieven <pl@kamp.de>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> 
> v1->v2:
>  Keep the 'flags' then use it(Base on Kevin's comments).

I think this patch wants a different subject line now.

> diff --git a/block/iscsi.c b/block/iscsi.c
> index 682abd8e09..50bae51700 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2002,7 +2002,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>          iscsilun->cluster_size = iscsilun->bl.opt_unmap_gran *
>              iscsilun->block_size;
>          if (iscsilun->lbprz) {
> -            ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
> +            ret = iscsi_allocmap_init(iscsilun, flags);
>          }
>      }

The code looks good.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Laurent Vivier March 10, 2020, 3:02 p.m. UTC | #2
Le 10/03/2020 à 15:26, Kevin Wolf a écrit :
> Am 02.03.2020 um 14:07 hat Chen Qun geschrieben:
>> Clang static code analyzer show warning:
>>   block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read
>>         flags &= ~BDRV_O_RDWR;
>>         ^        ~~~~~~~~~~~~
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>> ---
>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Peter Lieven <pl@kamp.de>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Max Reitz <mreitz@redhat.com>
>>
>> v1->v2:
>>  Keep the 'flags' then use it(Base on Kevin's comments).
> 
> I think this patch wants a different subject line now.


It needs also a better explanation in the commit message and should not
go through trivial as the change is not obvious.

Thanks,
Laurent

> 
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index 682abd8e09..50bae51700 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -2002,7 +2002,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
>>          iscsilun->cluster_size = iscsilun->bl.opt_unmap_gran *
>>              iscsilun->block_size;
>>          if (iscsilun->lbprz) {
>> -            ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
>> +            ret = iscsi_allocmap_init(iscsilun, flags);
>>          }
>>      }
> 
> The code looks good.
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> 
>
Chen Qun March 11, 2020, 2:04 a.m. UTC | #3
>-----Original Message-----
>From: Laurent Vivier [mailto:laurent@vivier.eu]
>Sent: Tuesday, March 10, 2020 11:02 PM
>To: Kevin Wolf <kwolf@redhat.com>; Chenqun (kuhn)
><kuhn.chenqun@huawei.com>
>Cc: peter.maydell@linaro.org; Zhanghailiang
><zhang.zhanghailiang@huawei.com>; qemu-trivial@nongnu.org; Peter Lieven
><pl@kamp.de>; qemu-devel@nongnu.org; Max Reitz <mreitz@redhat.com>;
>Ronnie Sahlberg <ronniesahlberg@gmail.com>; Euler Robot
><euler.robot@huawei.com>; Paolo Bonzini <pbonzini@redhat.com>
>Subject: Re: [PATCH v3 02/12] block/iscsi:Remove redundant statement in
>iscsi_open()
>
>Le 10/03/2020 à 15:26, Kevin Wolf a écrit :
>> Am 02.03.2020 um 14:07 hat Chen Qun geschrieben:
>>> Clang static code analyzer show warning:
>>>   block/iscsi.c:1920:9: warning: Value stored to 'flags' is never read
>>>         flags &= ~BDRV_O_RDWR;
>>>         ^        ~~~~~~~~~~~~
>>>
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>>> Signed-off-by: Chen Qun <kuhn.chenqun@huawei.com>
>>> ---
>>> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>> Cc: Peter Lieven <pl@kamp.de>
>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>> Cc: Max Reitz <mreitz@redhat.com>
>>>
>>> v1->v2:
>>>  Keep the 'flags' then use it(Base on Kevin's comments).
>>
>> I think this patch wants a different subject line now.
>
Yes,  it needs a more appropriate subject.

>It needs also a better explanation in the commit message and should not go
>through trivial as the change is not obvious.
>
OK , I will update the commit message base on existing comments.

Thanks.

>Thanks,
>Laurent
>
>>
>>> diff --git a/block/iscsi.c b/block/iscsi.c index
>>> 682abd8e09..50bae51700 100644
>>> --- a/block/iscsi.c
>>> +++ b/block/iscsi.c
>>> @@ -2002,7 +2002,7 @@ static int iscsi_open(BlockDriverState *bs, QDict
>*options, int flags,
>>>          iscsilun->cluster_size = iscsilun->bl.opt_unmap_gran *
>>>              iscsilun->block_size;
>>>          if (iscsilun->lbprz) {
>>> -            ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
>>> +            ret = iscsi_allocmap_init(iscsilun, flags);
>>>          }
>>>      }
>>
>> The code looks good.
>>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>
>>
diff mbox series

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index 682abd8e09..50bae51700 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2002,7 +2002,7 @@  static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         iscsilun->cluster_size = iscsilun->bl.opt_unmap_gran *
             iscsilun->block_size;
         if (iscsilun->lbprz) {
-            ret = iscsi_allocmap_init(iscsilun, bs->open_flags);
+            ret = iscsi_allocmap_init(iscsilun, flags);
         }
     }