diff mbox

[02/43] drivers/net/e2100.c: fix sparse warning: symbol shadows an earlier one

Message ID 20090214211203.23489.80420.stgit@vmbox.hanneseder.net
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Hannes Eder Feb. 14, 2009, 9:12 p.m. UTC 
Impact: Remove redundant inner scope variable and while being at it
make use of ARRAY_SIZE instead of a hardcoded number.

Fix this sparse warning:
  drivers/net/e2100.c:219:56: warning: symbol 'i' shadows an earlier one
  drivers/net/e2100.c:181:13: originally declared here

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---
 drivers/net/e2100.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Walter Harms Feb. 15, 2009, 2:04 p.m. UTC | #1 
Hi Hannes,
it is ok to replace a fixed boarder with ARRAY_SIZE()
in this special case i would success the following additional cleanup
(untestet !!)


static int set_free_irq(struct net_device *dev)
{
 int irqlist[] = {15,11,10,12,5,9,3,4}, i;

        for (i = 0; i < ARRAY_SIZE(irqlist); i++)
           if (request_irq (irqlist[i], NULL, 0, "bogus", NULL) != -EBUSY) {
                        dev->irq = irqlist[i];
                        return 0;
            }
        return -1;
}

....

if (dev->irq < 2) {
         if ( set_free_irq(dev) < 0) {
                printk(" unable to get IRQ %d.\n", dev->irq);
                retval = -EAGAIN;
                goto out;
        }

for me this looks more readable.

getting a free interrupt from a list mus be a common problem ? I am not in driver programming
perhaps such a code is already in place somewhere  ?

comments ?

re,
 wh


Hannes Eder schrieb:
> Impact: Remove redundant inner scope variable and while being at it
> make use of ARRAY_SIZE instead of a hardcoded number.
> 
> Fix this sparse warning:
>   drivers/net/e2100.c:219:56: warning: symbol 'i' shadows an earlier one
>   drivers/net/e2100.c:181:13: originally declared here
> 
> Signed-off-by: Hannes Eder <hannes@hanneseder.net>
> ---
>  drivers/net/e2100.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/e2100.c b/drivers/net/e2100.c
> index b07ba19..d2f6ee1 100644
> --- a/drivers/net/e2100.c
> +++ b/drivers/net/e2100.c
> @@ -216,13 +216,13 @@ static int __init e21_probe1(struct net_device *dev, int ioaddr)
>  		printk(" %02X", station_addr[i]);
>  
>  	if (dev->irq < 2) {
> -		int irqlist[] = {15,11,10,12,5,9,3,4}, i;
> -		for (i = 0; i < 8; i++)
> +		int irqlist[] = {15, 11, 10, 12, 5, 9, 3, 4};
> +		for (i = 0; i < ARRAY_SIZE(irqlist); i++)
>  			if (request_irq (irqlist[i], NULL, 0, "bogus", NULL) != -EBUSY) {
>  				dev->irq = irqlist[i];
>  				break;
>  			}
> -		if (i >= 8) {
> +		if (i >= ARRAY_SIZE(irqlist)) {
>  			printk(" unable to get IRQ %d.\n", dev->irq);
>  			retval = -EAGAIN;
>  			goto out;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Eder Feb. 15, 2009, 7:57 p.m. UTC | #2 
On Sun, Feb 15, 2009 at 3:04 PM, walter harms <wharms@bfs.de> wrote:
> Hi Hannes,
> it is ok to replace a fixed boarder with ARRAY_SIZE()
> in this special case i would success the following additional cleanup
> (untestet !!)
>
>
> static int set_free_irq(struct net_device *dev)
> {
>  int irqlist[] = {15,11,10,12,5,9,3,4}, i;
>
>        for (i = 0; i < ARRAY_SIZE(irqlist); i++)
>           if (request_irq (irqlist[i], NULL, 0, "bogus", NULL) != -EBUSY) {
>                        dev->irq = irqlist[i];
>                        return 0;
>            }
>        return -1;
> }
>
> ....
>
> if (dev->irq < 2) {
>         if ( set_free_irq(dev) < 0) {
>                printk(" unable to get IRQ %d.\n", dev->irq);
>                retval = -EAGAIN;
>                goto out;
>        }
>
> for me this looks more readable.
>
> getting a free interrupt from a list mus be a common problem ? I am not in driver programming
> perhaps such a code is already in place somewhere  ?

I am a great fan of refactoring but in that case I would just leave it
as is.  If extracting a method I would extract a little more here.

Best,
Hannes
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Walter Harms Feb. 16, 2009, 9:58 a.m. UTC | #3 
Hannes Eder schrieb:
> On Sun, Feb 15, 2009 at 3:04 PM, walter harms <wharms@bfs.de> wrote:
>> Hi Hannes,
>> it is ok to replace a fixed boarder with ARRAY_SIZE()
>> in this special case i would success the following additional cleanup
>> (untestet !!)
>>
>>
>> static int set_free_irq(struct net_device *dev)
>> {
>>  int irqlist[] = {15,11,10,12,5,9,3,4}, i;
>>
>>        for (i = 0; i < ARRAY_SIZE(irqlist); i++)
>>           if (request_irq (irqlist[i], NULL, 0, "bogus", NULL) != -EBUSY) {
>>                        dev->irq = irqlist[i];
>>                        return 0;
>>            }
>>        return -1;
>> }
>>
>> ....
>>
>> if (dev->irq < 2) {
>>         if ( set_free_irq(dev) < 0) {
>>                printk(" unable to get IRQ %d.\n", dev->irq);
>>                retval = -EAGAIN;
>>                goto out;
>>        }
>>
>> for me this looks more readable.
>>
>> getting a free interrupt from a list mus be a common problem ? I am not in driver programming
>> perhaps such a code is already in place somewhere  ?
> 
> I am a great fan of refactoring but in that case I would just leave it
> as is.  If extracting a method I would extract a little more here.
> 

Me too,
i hope the driver maintainer reads it and will react accordingly.

re,
 wh

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Feb. 18, 2009, 1:21 a.m. UTC | #4 
From: Hannes Eder <hannes@hanneseder.net>
Date: Sat, 14 Feb 2009 22:12:10 +0100

> Impact: Remove redundant inner scope variable and while being at it
> make use of ARRAY_SIZE instead of a hardcoded number.
> 
> Fix this sparse warning:
>   drivers/net/e2100.c:219:56: warning: symbol 'i' shadows an earlier one
>   drivers/net/e2100.c:181:13: originally declared here
> 
> Signed-off-by: Hannes Eder <hannes@hanneseder.net>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/e2100.c b/drivers/net/e2100.c
index b07ba19..d2f6ee1 100644
--- a/drivers/net/e2100.c
+++ b/drivers/net/e2100.c
@@ -216,13 +216,13 @@  static int __init e21_probe1(struct net_device *dev, int ioaddr)
 		printk(" %02X", station_addr[i]);
 
 	if (dev->irq < 2) {
-		int irqlist[] = {15,11,10,12,5,9,3,4}, i;
-		for (i = 0; i < 8; i++)
+		int irqlist[] = {15, 11, 10, 12, 5, 9, 3, 4};
+		for (i = 0; i < ARRAY_SIZE(irqlist); i++)
 			if (request_irq (irqlist[i], NULL, 0, "bogus", NULL) != -EBUSY) {
 				dev->irq = irqlist[i];
 				break;
 			}
-		if (i >= 8) {
+		if (i >= ARRAY_SIZE(irqlist)) {
 			printk(" unable to get IRQ %d.\n", dev->irq);
 			retval = -EAGAIN;
 			goto out;