[PATCH resubmit] /drivers/ata ioremap returncode check

[PATCH resubmit] /drivers/ata ioremap returncode check

[Scott Thompson]

Patchset against 2.6.23-rc3.  corrects missing ioremap return 
checks, resending after making changes suggested....

Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
------------------------------------------------------------
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 4ca7fd6..8dc7c3b 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
 	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
 	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
 
+	if (!data->cs0 || !data->cs1) {
+		return -ENOMEM;
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq)
 		set_irq_type(irq, IRQT_RISING);

> ----- Forwarded message from Brandon Philips <brandon@ifup.org> ----
> -
> The iounmap calls are unnecessary since devm_ioremap will un-
> allocate the space if you return an error from probe.  See
> Documentation/driver-model/devres.txt
> 
> But, something like this is needed.
> 
> +	if (!data->cs0 || !data->cs1)
> +		return -ENOMEM;
> 
> Thanks,	Brandon




       
____________________________________________________________________________________
Pinpoint customers who are looking for what you sell. 
http://searchmarketing.yahoo.com/


       
____________________________________________________________________________________
Building a website is a piece of cake. Yahoo! Small Business gives you all the tools to get online.
http://smallbusiness.yahoo.com/webhosting 

Re: [PATCH resubmit] /drivers/ata ioremap returncode check

[Brandon Philips]

On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> Patchset against 2.6.23-rc3.  corrects missing ioremap return 
> checks, resending after making changes suggested....
> 
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> ------------------------------------------------------------
> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> index 4ca7fd6..8dc7c3b 100644
> --- a/drivers/ata/pata_ixp4xx_cf.c
> +++ b/drivers/ata/pata_ixp4xx_cf.c
> @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
>  	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
>  	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
>  
> +	if (!data->cs0 || !data->cs1) {
> +		return -ENOMEM;
> +	}
> +

You aren't following the Kernel CodingStyle there.  

See Documentation/CodingStyle:

"Do not unnecessarily use braces where a single statement will do."

+	if (!data->cs0 || !data->cs1)
+		return -ENOMEM;

Thanks,

	Brandon

Re: [PATCH resubmit] /drivers/ata ioremap returncode check

[Alexey Dobriyan]

On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > Patchset against 2.6.23-rc3.  corrects missing ioremap return 
> > checks, resending after making changes suggested....
> > 
> > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > ------------------------------------------------------------
> > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > index 4ca7fd6..8dc7c3b 100644
> > --- a/drivers/ata/pata_ixp4xx_cf.c
> > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> >  	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> >  	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> >  
> > +	if (!data->cs0 || !data->cs1) {
> > +		return -ENOMEM;
> > +	}
> > +
> 
> You aren't following the Kernel CodingStyle there.  
> 
> See Documentation/CodingStyle:
> 
> "Do not unnecessarily use braces where a single statement will do."
> 
> +	if (!data->cs0 || !data->cs1)
> +		return -ENOMEM;

This and nice printk message absence. :)

Re: [PATCH resubmit] /drivers/ata ioremap returncode check

[Brandon Philips]

On 21:42 Thu 23 Aug 2007, Alexey Dobriyan wrote:
> On Wed, Aug 22, 2007 at 10:34:58PM -0700, Brandon Philips wrote:
> > On 19:47 Wed 22 Aug 2007, Scott Thompson wrote:
> > > Patchset against 2.6.23-rc3.  corrects missing ioremap return 
> > > checks, resending after making changes suggested....
> > > 
> > > Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
> > > ------------------------------------------------------------
> > > diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> > > index 4ca7fd6..8dc7c3b 100644
> > > --- a/drivers/ata/pata_ixp4xx_cf.c
> > > +++ b/drivers/ata/pata_ixp4xx_cf.c
> > > @@ -189,6 +189,10 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
> > >  	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> > >  	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> > >  
> > > +	if (!data->cs0 || !data->cs1) {
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > 
> > You aren't following the Kernel CodingStyle there.  
> > 
> > See Documentation/CodingStyle:
> > 
> > "Do not unnecessarily use braces where a single statement will do."
> > 
> > +	if (!data->cs0 || !data->cs1)
> > +		return -ENOMEM;
> 
> This and nice printk message absence. :)

Huh?  What do you mean?

Thanks,

	Brandon

[PATCH resubmit] /drivers/ata ioremap returncode check

[Scott Thompson]

Patchset against 2.6.23-rc3.  corrects missing ioremap return 
checks, resending after fixing for documentation making changes suggested...

As for suggestions, I did fix the {} that weren't needed.  I did not add in a
"printk" err or warn statement as the rest of the module does not contain these
messages.  When auditing (as I am for ioremap checking here) I am attempting
to keep changes "in the style of their surroundings".  Otherwise it's massive
scope screep on these patches.
 
Signed-off-by: Scott Thompson <postfail <at> hushmail.com>
---------------------------------------------------------------
diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
index 4ca7fd6..6f9a2b7 100644
--- a/drivers/ata/pata_ixp4xx_cf.c
+++ b/drivers/ata/pata_ixp4xx_cf.c
@@ -189,6 +189,9 @@ static __devinit int ixp4xx_pata_probe(struct platform_device *pdev)
 	data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
 	data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
 
+	if (!data->cs0 || !data->cs1) 
+		return -ENOMEM;
+	
 	irq = platform_get_irq(pdev, 0);
 	if (irq)
 		set_irq_type(irq, IRQT_RISING);


       
____________________________________________________________________________________
Moody friends. Drama queens. Your life? Nope! - their life, your story. Play Sims Stories at
Yahoo! Games.
http://sims.yahoo.com/  


       
____________________________________________________________________________________
Looking for a deal? Find great prices on flights and hotels with Yahoo! FareChase.
http://farechase.yahoo.com/

Re: [PATCH resubmit] /drivers/ata ioremap returncode check

[Tejun Heo]

Scott Thompson wrote:
> Patchset against 2.6.23-rc3.  corrects missing ioremap return 
> checks, resending after fixing for documentation making changes suggested...
> 
> As for suggestions, I did fix the {} that weren't needed.  I did not add in a
> "printk" err or warn statement as the rest of the module does not contain these
> messages.  When auditing (as I am for ioremap checking here) I am attempting
> to keep changes "in the style of their surroundings".  Otherwise it's massive
> scope screep on these patches.
>  
> Signed-off-by: Scott Thompson <postfail <at> hushmail.com>

Acked-by: Tejun Heo <htejun@gmail.com>

It would be better to start the subject line with pata_ixp4xx_cf: but
maybe Jeff can fix it up while applying.

Thanks.

-- 
tejun