Re: [PATCH v1 1/1] ufs: mcq: qcom: Fix Smatch static checker warning

[Asutosh Das]

On Wed, Mar 01 2023 at 23:05 -0800, Manivannan Sadhasivam wrote:
> On Tue, Feb 28, 2023 at 10:27:06AM -0800, Asutosh Das wrote:
> > The patch (c263b4ef737e: "scsi: ufs: core: mcq: Configure resource
> > regions") from Jan 13, 2023, leads to the following Smatch static
> > checker warning:
> >
> > drivers/ufs/host/ufs-qcom.c:1455 ufs_qcom_mcq_config_resource() warn:
> > passing zero to 'PTR_ERR'
> > drivers/ufs/host/ufs-qcom.c:1469 ufs_qcom_mcq_config_resource() info:
> > returning a literal zero is cleaner
> >
> > Fix the above warnings.
>
> You should not name the subject after the tool that used to find the issues.
> Instead, subject should mention the actual issue like fixing return values,
> removing the devm_kfree in error path etc...
>
> Provided that, you should also split this patch into two as you are fixing
> two independent issues.
>
> Thanks,
> Mani

Thanks for taking a look.
I will fix it in the next version.

-Asd

> > Fixes: c263b4ef737e ("scsi: ufs: core: mcq: Configure resource regions")
> > Reported-by: Dan Carpenter <error27@xxxxxxxxx>
> > Signed-off-by: Asutosh Das <quic_asutoshd@xxxxxxxxxxx>
> > ---
> >  drivers/ufs/host/ufs-qcom.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/ufs/host/ufs-qcom.c b/drivers/ufs/host/ufs-qcom.c
> > index 34fc453f3eb1..cb20c7136c2c 100644
> > --- a/drivers/ufs/host/ufs-qcom.c
> > +++ b/drivers/ufs/host/ufs-qcom.c
> > @@ -1451,8 +1451,8 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba)
> >  		if (IS_ERR(res->base)) {
> >  			dev_err(hba->dev, "Failed to map res %s, err=%d\n",
> >  					 res->name, (int)PTR_ERR(res->base));
> > -			res->base = NULL;
> >  			ret = PTR_ERR(res->base);
> > +			res->base = NULL;
> >  			return ret;
> >  		}
> >  	}
> > @@ -1466,7 +1466,7 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba)
> >  	/* Explicitly allocate MCQ resource from ufs_mem */
> >  	res_mcq = devm_kzalloc(hba->dev, sizeof(*res_mcq), GFP_KERNEL);
> >  	if (!res_mcq)
> > -		return ret;
> > +		return -ENOMEM;
> >
> >  	res_mcq->start = res_mem->start +
> >  			 MCQ_SQATTR_OFFSET(hba->mcq_capabilities);
> > @@ -1478,7 +1478,7 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba)
> >  	if (ret) {
> >  		dev_err(hba->dev, "Failed to insert MCQ resource, err=%d\n",
> >  			ret);
> > -		goto insert_res_err;
> > +		return ret;
> >  	}
> >
> >  	res->base = devm_ioremap_resource(hba->dev, res_mcq);
> > @@ -1495,8 +1495,6 @@ static int ufs_qcom_mcq_config_resource(struct ufs_hba *hba)
> >  ioremap_err:
> >  	res->base = NULL;
> >  	remove_resource(res_mcq);
> > -insert_res_err:
> > -	devm_kfree(hba->dev, res_mcq);
> >  	return ret;
> >  }
> >
> > --
> > 2.7.4
>
> --
> மணிவண்ணன் சதாசிவம்