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

From: walter harms <[email-masked]>
Date: Mon, 16 Feb 2009 10:58:06 +0100

Hannes Eder schrieb:
> On Sun, Feb 15, 2009 at 3:04 PM, walter harms <[email-masked]> 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