On Wed, Jan 11, 2006 at 03:48:11PM +0100, Rogier Wolff wrote:
> On Wed, Jan 11, 2006 at 03:11:47PM +0100, Eric Dumazet wrote:
> > Rogier Wolff a écrit :
> > >On Wed, Jan 11, 2006 at 02:43:49PM +0100, Erik Mouw wrote:
> > >>The system only recovers after the Netdev watchdog found out that the
> > >>transmit timed out. However, the e1000 register dump starts about 4 to
> > >>5 seconds earlier: a possible workaround would be to trigger the
> > >>timeout code path as soon as the register dump starts.
> > >
> > >Found a typo.
> > >
> > > Roger.
> > >
> > >
> > >--- e1000_main.c.orig 2006-01-11 14:53:23.000000000 +0100
> > >+++ e1000_main.c 2006-01-11 14:53:38.000000000 +0100
> > >@@ -3449,7 +3449,7 @@
> > > }
> > >
> > > for (i = 0; i < E1000_MAX_INTR; i++)
> > >- if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &
> > >+ if (unlikely(!adapter->clean_rx(adapter, adapter->rx_ring) &&
> > > !e1000_clean_tx_irq(adapter, adapter->tx_ring)))
> > > break;
> > >
> > >
> > >
> >
> > I believe it's not a typo.
> >
> > The intention is to call both clean_rx() and e1000_clean_tx_irq(), and
> > break of the loop if both said : There was no job pending.
>
> Although (one of) the prototypes state(s) that it returns a boolean,
> it is valid C to return the number of items pending.
>
> And if one reports "2 more pending" and the other reports "1 more
> pending", the "&" between the two becomes 2 & 1 => 0 / FALSE, while 2
> && 1 => TRUE.
>
> I consider this a low prio typo, not likely to be a bug "right now",
> but it is inviting someone to make it into a bug later on.
Oops. The line itself has "logical NOT" operators. So indeed it is NOW
not a bug, and slighlty less likely to become a bug later on, but
still bad programming practise.
Roger.
--
+-- Rogier Wolff -- www.harddisk-recovery.nl -- 0800 220 20 20 --
| Files foetsie, bestanden kwijt, alle data weg?!
| Blijf kalm en neem contact op met Harddisk-recovery.nl!
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
