Re: ixgb boolean typo ?

[Herbert Xu]

Andrew Morton <akpm@xxxxxxxx> wrote:
> Dave Jones <davej@xxxxxxxxxx> wrote:
>>
>> Is this perhaps what was meant to be here ?
>> 
>>               Dave
>> 
>>  --- 1/drivers/net/ixgb/ixgb_main.c~  2004-08-03 01:18:00.000000000 +0100
>>  +++ 2/drivers/net/ixgb/ixgb_main.c   2004-08-03 01:53:46.653695768 +0100
>>  @@ -1615,7 +1615,7 @@
>>       }
>>   #else
>>       for (i = 0; i < IXGB_MAX_INTR; i++)
>>  -            if (!ixgb_clean_rx_irq(adapter) & !ixgb_clean_tx_irq(adapter))
>>  +            if (!ixgb_clean_rx_irq(adapter) && !ixgb_clean_tx_irq(adapter))
>>                       break;
>>       /* if RAIDC:EN == 1 and ICR:RXDMT0 == 1, we need to
>>        * set IMS:RXDMT0 to 1 to restart the RBD timer (POLL)
> 
> Both versions will do the same thing, inefficiently: keep going until both
> functions return false.  And keep pointlessly calling a functions which is
> returning FALSE all the time.
> 
> I think this would be better:

Please note that ixgb_clean_rx_irq/ixgb_clean_tx_irq are not pure functions.
Their return values can change.

The original intention appears to be this: keep processing until both
RX and TX runs out of things to do.

Both of your patches do something different.

Perhaps we should add a comment instead.
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/