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

Chen Qun posted 12 patches 5 years, 6 months ago
Maintainers: Peter Lieven <pl@kamp.de>, John Snow <jsnow@redhat.com>, Max Reitz <mreitz@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Kevin Wolf <kwolf@redhat.com>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Alistair Francis <alistair@alistair23.me>, Igor Mitsyanko <i.mitsyanko@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Ronnie Sahlberg <ronniesahlberg@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, Fam Zheng <fam@euphon.net>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v3 02/12] block/iscsi:Remove redundant statement in iscsi_open()
Posted by Chen Qun 5 years, 6 months ago
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(-)

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);
         }
     }
 
-- 
2.23.0



Re: [PATCH v3 02/12] block/iscsi:Remove redundant statement in iscsi_open()
Posted by Kevin Wolf 5 years, 6 months ago
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>


Re: [PATCH v3 02/12] block/iscsi:Remove redundant statement in iscsi_open()
Posted by Laurent Vivier 5 years, 6 months ago
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>
> 
> 


RE: [PATCH v3 02/12] block/iscsi:Remove redundant statement in iscsi_open()
Posted by Chenqun (kuhn) 5 years, 5 months ago
>-----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>
>>
>>