Re: [Patch] Signedness issue in drivers/net/3c515.c

2 messages, 2 authors, 2006-08-21 · open the first message on its own page

Re: [Patch] Signedness issue in drivers/net/3c515.c

From: Andrew Morton <hidden>
Date: 2006-08-21 21:06:12

On Sat, 19 Aug 2006 19:37:57 +0200
Eric Sesterhenn [off-list ref] wrote:
quoted hunk
while playing with gcc 4.1 -Wextra warnings, I came across this one:

drivers/net/3c515.c:1027: warning: comparison of unsigned expression >= 0 is always true

Since i is unsigned the >= 0 check in the for loop is always true,
so we might spin there forever unless the if condition triggers.
Since i is only used in this loop, this patch changes it to
an integer.

Signed-off-by: Eric Sesterhenn <redacted>
--- linux-2.6.18-rc4/drivers/net/3c515.c.orig	2006-08-19 19:35:04.000000000 +0200
+++ linux-2.6.18-rc4/drivers/net/3c515.c	2006-08-19 19:35:14.000000000 +0200
@@ -1003,7 +1003,8 @@ static int corkscrew_start_xmit(struct s
 		/* Calculate the next Tx descriptor entry. */
 		int entry = vp->cur_tx % TX_RING_SIZE;
 		struct boom_tx_desc *prev_entry;
-		unsigned long flags, i;
+		unsigned long flags;
+		int i;
 
 		if (vp->tx_full)	/* No room to transmit with */
 			return 1;
Which affects this loop:

	/* Wait for the stall to complete. */
	for (i = 20; i >= 0; i--)
		if ((inw(ioaddr + EL3_STATUS) & CmdInProgress) == 0) 
			break;

Your fix will convert this indefinit wait into a bounded one.  It might
cause the driver to malfunction.

Given that our pool of 3c515 testers is less than enormous, a more prudent
change might be to remove `i' and simply formalise the existing behaviour
into a while(1) loop.

Re: [Patch] Signedness issue in drivers/net/3c515.c

From: Alan Cox <hidden>
Date: 2006-08-21 21:52:56

Ar Llu, 2006-08-21 am 14:05 -0700, ysgrifennodd Andrew Morton:
	/* Wait for the stall to complete. */
	for (i = 20; i >= 0; i--)
		if ((inw(ioaddr + EL3_STATUS) & CmdInProgress) == 0) 
			break;

Your fix will convert this indefinit wait into a bounded one.  It might
cause the driver to malfunction.
The change is correct. The docs guarantee it can't take that long.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help