[PATCH] advansys: Remove redundant assignment to err and n_q_required

[PATCH] advansys: Remove redundant assignment to err and n_q_required

[Jiapeng Chong]

Variable err and n_q_required is set to '-ENOMEM' and '1', but they are
either overwritten or unused later on, so these are redundant assignments
that can be removed.

Clean up the following clang-analyzer warning:

drivers/scsi/advansys.c:11235:2: warning: Value stored to 'err' is never
read [clang-analyzer-deadcode.DeadStores].

drivers/scsi/advansys.c:8091:2: warning: Value stored to 'n_q_required'
is never read [clang-analyzer-deadcode.DeadStores].

drivers/scsi/advansys.c:11484:2: warning: Value stored to 'err' is never
read [clang-analyzer-deadcode.DeadStores].

Reported-by: Abaci Robot <abaci@linux.alibaba.com>
Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
---
 drivers/scsi/advansys.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 800052f..f9969d4 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -8088,7 +8088,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq)
 	sta = 0;
 	target_ix = scsiq->q2.target_ix;
 	tid_no = ASC_TIX_TO_TID(target_ix);
-	n_q_required = 1;
 	if (scsiq->cdbptr[0] == REQUEST_SENSE) {
 		if ((asc_dvc->init_sdtr & scsiq->q1.target_id) != 0) {
 			asc_dvc->sdtr_done &= ~scsiq->q1.target_id;
@@ -11232,7 +11231,6 @@ static int advansys_vlb_probe(struct device *dev, unsigned int id)
 	if (AscGetChipVersion(iop_base, ASC_IS_VL) > ASC_CHIP_MAX_VER_VL)
 		goto release_region;
 
-	err = -ENOMEM;
 	shost = scsi_host_alloc(&advansys_template, sizeof(*board));
 	if (!shost)
 		goto release_region;
@@ -11457,7 +11455,6 @@ static int advansys_pci_probe(struct pci_dev *pdev,
 
 	ioport = pci_resource_start(pdev, 0);
 
-	err = -ENOMEM;
 	shost = scsi_host_alloc(&advansys_template, sizeof(*board));
 	if (!shost)
 		goto release_region;
-- 
1.8.3.1

Re: [PATCH] advansys: Remove redundant assignment to err and n_q_required

[Matthew Wilcox]

On Fri, Apr 30, 2021 at 05:25:28PM +0800, Jiapeng Chong wrote:
> Variable err and n_q_required is set to '-ENOMEM' and '1', but they are
> either overwritten or unused later on, so these are redundant assignments
> that can be removed.
> 
> Clean up the following clang-analyzer warning:
> 
> drivers/scsi/advansys.c:11235:2: warning: Value stored to 'err' is never
> read [clang-analyzer-deadcode.DeadStores].
> 
> drivers/scsi/advansys.c:8091:2: warning: Value stored to 'n_q_required'
> is never read [clang-analyzer-deadcode.DeadStores].
> 
> drivers/scsi/advansys.c:11484:2: warning: Value stored to 'err' is never
> read [clang-analyzer-deadcode.DeadStores].

I don't want to spend any time figuring out if this is a legitimate patch
or not.  Please stop running these analysers on this driver, and thank
the University of Minnesota for making me suspicious.

Re: [PATCH] advansys: Remove redundant assignment to err and n_q_required

[Finn Thain]

On Fri, 30 Apr 2021, Jiapeng Chong wrote:

> Variable err and n_q_required is set to '-ENOMEM' and '1', but they are
> either overwritten or unused later on, so these are redundant assignments
> that can be removed.
> 
> Clean up the following clang-analyzer warning:
> 
> drivers/scsi/advansys.c:11235:2: warning: Value stored to 'err' is never
> read [clang-analyzer-deadcode.DeadStores].
> 
> drivers/scsi/advansys.c:8091:2: warning: Value stored to 'n_q_required'
> is never read [clang-analyzer-deadcode.DeadStores].
> 
> drivers/scsi/advansys.c:11484:2: warning: Value stored to 'err' is never
> read [clang-analyzer-deadcode.DeadStores].
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
> ---
>  drivers/scsi/advansys.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
> index 800052f..f9969d4 100644
> --- a/drivers/scsi/advansys.c
> +++ b/drivers/scsi/advansys.c
> @@ -8088,7 +8088,6 @@ static int AscExeScsiQueue(ASC_DVC_VAR *asc_dvc, ASC_SCSI_Q *scsiq)
>  	sta = 0;
>  	target_ix = scsiq->q2.target_ix;
>  	tid_no = ASC_TIX_TO_TID(target_ix);
> -	n_q_required = 1;
>  	if (scsiq->cdbptr[0] == REQUEST_SENSE) {
>  		if ((asc_dvc->init_sdtr & scsiq->q1.target_id) != 0) {
>  			asc_dvc->sdtr_done &= ~scsiq->q1.target_id;
> @@ -11232,7 +11231,6 @@ static int advansys_vlb_probe(struct device *dev, unsigned int id)
>  	if (AscGetChipVersion(iop_base, ASC_IS_VL) > ASC_CHIP_MAX_VER_VL)
>  		goto release_region;
>  
> -	err = -ENOMEM;
>  	shost = scsi_host_alloc(&advansys_template, sizeof(*board));
>  	if (!shost)
>  		goto release_region;

No, the correct way to resolve that particular clang warning is,

  free_host:
         scsi_host_put(shost);
  release_region:
         release_region(iop_base, ASC_IOADR_GAP);
-        return -ENODEV;
+        return err;
 }

> @@ -11457,7 +11455,6 @@ static int advansys_pci_probe(struct pci_dev *pdev,
>  
>  	ioport = pci_resource_start(pdev, 0);
>  
> -	err = -ENOMEM;
>  	shost = scsi_host_alloc(&advansys_template, sizeof(*board));
>  	if (!shost)
>  		goto release_region;
> 

No, that would be a behavioural change for the error path.

Moreover, the clang warning you claimed for line 11484 appears to be bogus 
with regard to mainline code. You may want to check your checker.