Patchwork edac: qcom_edac: uninitialized variable in IRQ handler

login
register
mail settings
Submitter Dan Carpenter
Date Oct. 18, 2018, 7:38 a.m.
Message ID <20181018073807.en2p4tt3qkmdnus6@kili.mountain>
Download mbox | patch
Permalink /patch/638593/
State New
Headers show

Comments

Dan Carpenter - Oct. 18, 2018, 7:38 a.m.
We forgot to set "irq_handled" to false, so it's either true or
uninitialized.

Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/edac/qcom_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Borislav Petkov - Oct. 18, 2018, 12:27 p.m.
On Thu, Oct 18, 2018 at 10:38:07AM +0300, Dan Carpenter wrote:
> We forgot to set "irq_handled" to false, so it's either true or
> uninitialized.
> 
> Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/edac/qcom_edac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> index 82bd775124f2..f8510a024fe5 100644
> --- a/drivers/edac/qcom_edac.c
> +++ b/drivers/edac/qcom_edac.c
> @@ -292,7 +292,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
>  	struct llcc_drv_data *drv = edac_dev_ctl->pvt_info;
>  	irqreturn_t irq_rc = IRQ_NONE;
>  	u32 drp_error, trp_error, i;
> -	bool irq_handled;
> +	bool irq_handled = false;

That variable is just silly. One can simply do:

	if (!ret)
		irq_rc = IRQ_HANDLED;

in both places and

	return irq_rc;

at the end and remove irq_handled altogether. Problem solved.
Dan Carpenter - Oct. 18, 2018, 12:32 p.m.
On Thu, Oct 18, 2018 at 02:27:14PM +0200, Borislav Petkov wrote:
> On Thu, Oct 18, 2018 at 10:38:07AM +0300, Dan Carpenter wrote:
> > We forgot to set "irq_handled" to false, so it's either true or
> > uninitialized.
> > 
> > Fixes: 27450653f1db ("drivers: edac: Add EDAC driver support for QCOM SoCs")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> >  drivers/edac/qcom_edac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
> > index 82bd775124f2..f8510a024fe5 100644
> > --- a/drivers/edac/qcom_edac.c
> > +++ b/drivers/edac/qcom_edac.c
> > @@ -292,7 +292,7 @@ llcc_ecc_irq_handler(int irq, void *edev_ctl)
> >  	struct llcc_drv_data *drv = edac_dev_ctl->pvt_info;
> >  	irqreturn_t irq_rc = IRQ_NONE;
> >  	u32 drp_error, trp_error, i;
> > -	bool irq_handled;
> > +	bool irq_handled = false;
> 
> That variable is just silly. One can simply do:
> 
> 	if (!ret)
> 		irq_rc = IRQ_HANDLED;
> 
> in both places and
> 
> 	return irq_rc;
> 
> at the end and remove irq_handled altogether. Problem solved.
> 

Yeah.  That's cleaner.  I will resend.

regards,
dan carpenter

Patch

diff --git a/drivers/edac/qcom_edac.c b/drivers/edac/qcom_edac.c
index 82bd775124f2..f8510a024fe5 100644
--- a/drivers/edac/qcom_edac.c
+++ b/drivers/edac/qcom_edac.c
@@ -292,7 +292,7 @@  llcc_ecc_irq_handler(int irq, void *edev_ctl)
 	struct llcc_drv_data *drv = edac_dev_ctl->pvt_info;
 	irqreturn_t irq_rc = IRQ_NONE;
 	u32 drp_error, trp_error, i;
-	bool irq_handled;
+	bool irq_handled = false;
 	int ret;
 
 	/* Iterate over the banks and look for Tag RAM or Data RAM errors */