| Message ID | 20231113050147.80818-1-suhui@nfschina.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: pm8001: return error code if no attached dev | expand |
Hi, Su On 2023/11/13 13:01, Su Hui wrote: > Clang static analyzer complains that value stored to 'res' is never read. > Return the error code when sas_find_attached_phy_id() failed. > > Fixes: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() instead of open coding it") This 'Fixes' tag is not right. This commit is only a refactor and did not change the original logic here. Thanks, Jason > Signed-off-by: Su Hui <suhui@nfschina.com> > --- > drivers/scsi/pm8001/pm8001_sas.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index a5a31dfa4512..a1f58bfff5c0 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -662,6 +662,8 @@ static int pm8001_dev_found_notify(struct domain_device *dev) > SAS_ADDR(dev->sas_addr), > SAS_ADDR(parent_dev->sas_addr)); > res = phy_id; > + pm8001_free_dev(pm8001_device); > + goto found_out; > } else { > pm8001_device->attached_phy = phy_id; > } >
On 2023/11/13 13:56, Jason Yan wrote: > Hi, Su > > On 2023/11/13 13:01, Su Hui wrote: >> Clang static analyzer complains that value stored to 'res' is never >> read. >> Return the error code when sas_find_attached_phy_id() failed. >> >> Fixes: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() >> instead of open coding it") > > This 'Fixes' tag is not right. This commit is only a refactor and did > not change the original logic here. > Hi, Jason Thanks for your reply. But I think the logic of this code is changed. >> Signed-off-by: Su Hui <suhui@nfschina.com> >> --- >> drivers/scsi/pm8001/pm8001_sas.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/scsi/pm8001/pm8001_sas.c >> b/drivers/scsi/pm8001/pm8001_sas.c >> index a5a31dfa4512..a1f58bfff5c0 100644 >> --- a/drivers/scsi/pm8001/pm8001_sas.c >> +++ b/drivers/scsi/pm8001/pm8001_sas.c >> @@ -662,6 +662,8 @@ static int pm8001_dev_found_notify(struct >> domain_device *dev) >> SAS_ADDR(dev->sas_addr), >> SAS_ADDR(parent_dev->sas_addr)); >> res = phy_id; >> + pm8001_free_dev(pm8001_device); >> + goto found_out; Before this patch, we won't go to label 'found_out', and will call PM8001_CHIP_DISP->reg_dev_req() .... After this patch, we just free the 'pm8001_device' and return the error code. Or maybe I misunderstand somewhere? Su Hui
On 2023/11/13 14:32, Su Hui wrote: > On 2023/11/13 13:56, Jason Yan wrote: >> Hi, Su >> >> On 2023/11/13 13:01, Su Hui wrote: >>> Clang static analyzer complains that value stored to 'res' is never >>> read. >>> Return the error code when sas_find_attached_phy_id() failed. >>> >>> Fixes: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() >>> instead of open coding it") >> >> This 'Fixes' tag is not right. This commit is only a refactor and did >> not change the original logic here. >> > Hi, Jason > > Thanks for your reply. But I think the logic of this code is changed. I,m talking about the Fixes tag: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() instead of open coding it" That commit did not change the original logic. So your patch is not fixing that commit. > >>> Signed-off-by: Su Hui <suhui@nfschina.com> >>> --- >>> drivers/scsi/pm8001/pm8001_sas.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/scsi/pm8001/pm8001_sas.c >>> b/drivers/scsi/pm8001/pm8001_sas.c >>> index a5a31dfa4512..a1f58bfff5c0 100644 >>> --- a/drivers/scsi/pm8001/pm8001_sas.c >>> +++ b/drivers/scsi/pm8001/pm8001_sas.c >>> @@ -662,6 +662,8 @@ static int pm8001_dev_found_notify(struct >>> domain_device *dev) >>> SAS_ADDR(dev->sas_addr), >>> SAS_ADDR(parent_dev->sas_addr)); >>> res = phy_id; >>> + pm8001_free_dev(pm8001_device); >>> + goto found_out; > > Before this patch, we won't go to label 'found_out', and will call > PM8001_CHIP_DISP->reg_dev_req() .... > > After this patch, we just free the 'pm8001_device' and return the error > code. > > Or maybe I misunderstand somewhere? Sorry, but I'm not talking about your patch. Thanks, Jason > > Su Hui > > > .
On 2023/11/13 14:43, Jason Yan wrote: > On 2023/11/13 14:32, Su Hui wrote: >> On 2023/11/13 13:56, Jason Yan wrote: >>> Hi, Su >>> >>> On 2023/11/13 13:01, Su Hui wrote: >>>> Clang static analyzer complains that value stored to 'res' is never >>>> read. >>>> Return the error code when sas_find_attached_phy_id() failed. >>>> >>>> Fixes: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() >>>> instead of open coding it") >>> >>> This 'Fixes' tag is not right. This commit is only a refactor and >>> did not change the original logic here. >>> >> Hi, Jason >> >> Thanks for your reply. But I think the logic of this code is changed. > > I,m talking about the Fixes tag: ec64858657a8 ("scsi: pm8001: Use > sas_find_attached_phy_id() instead of open coding it" > > That commit did not change the original logic. So your patch is not > fixing that commit. Oh, got it. Really thanks for your reminder! I will send v2 patch soon:). Su Hui
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index a5a31dfa4512..a1f58bfff5c0 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -662,6 +662,8 @@ static int pm8001_dev_found_notify(struct domain_device *dev) SAS_ADDR(dev->sas_addr), SAS_ADDR(parent_dev->sas_addr)); res = phy_id; + pm8001_free_dev(pm8001_device); + goto found_out; } else { pm8001_device->attached_phy = phy_id; }
Clang static analyzer complains that value stored to 'res' is never read. Return the error code when sas_find_attached_phy_id() failed. Fixes: ec64858657a8 ("scsi: pm8001: Use sas_find_attached_phy_id() instead of open coding it") Signed-off-by: Su Hui <suhui@nfschina.com> --- drivers/scsi/pm8001/pm8001_sas.c | 2 ++ 1 file changed, 2 insertions(+)