ehci tests unsigned variables against 0

I found 63 occurrences of this problem with the following semantic match
(http://www.emn.fr/x-info/coccinelle/):

@@ unsigned int i; @@

* i < 0

I looked through all of the results by hand, and they all seem to be problems. In many cases, it seems like the variable should not be unsigned as it is used to hold the return value of a function that might return a negative error code, but I haven't looked into this in detail.

In the output below, the lines that begin with a single start contain a test of whether an unsigned variable or structure field is less than 0. The output is actually generated with diff, but I converted the -s to *s to avoid confusion.

diff -u -p a/drivers/usb/host/ehci-dbg.c b/drivers/usb/host/ehci-dbg.c *** a/drivers/usb/host/ehci-dbg.c 2008-03-12 14:13:14.000000000 +0100

(quoted diff) +0 -0

@@ -454,7 +454,7 @@ static void qh_lines (
 				(scratch >> 16) & 0x7fff,
 				scratch,
 				td->urb);
 *		if (temp < 0)
 			temp = 0;
 		else if (size < temp)
 			temp = size;

These tests will never trigger and should simply be removed.  Linux's kernel snprintf function conforms to C99 and never returns 0.

Quoted 2 lines · Matthew Wilcox

These tests will never trigger and should simply be removed. ?Linux's kernel snprintf function conforms to C99 and never returns 0.

I think you mean "never returns negative" ?

-ENOPATCH

... although if that's the case, I'd think that the *snprintf() signatures are incorrect: they should return "unsigned" not "int". If that were done, I think even GCC could be made to report such issues; one wouldn't need less-common tools like coccinelle.

(What I've seen of coccinelle makes me glad it's being applied to the kernel sources, by the way.)

Quoted 1 line

-ENOPATCH

Yeah ... I didn't bother with one. Let's try now:

drivers/usb/host/ehci-dbg.c +2 -6

index 64ebfc5..55a2c73 100644
@@ -454,9 +454,7 @@ static void qh_lines (
 				(scratch >> 16) & 0x7fff,
 				scratch,
 				td->urb);
-		if (temp < 0)
-			temp = 0;
-		else if (size < temp)
+		if (size < temp)
 			temp = size;
 		size -= temp;
 		next += temp;

@@ -465,9 +463,7 @@ static void qh_lines (
 	}
 
 	temp = snprintf (next, size, "\n");
-	if (temp < 0)
-		temp = 0;
-	else if (size < temp)
+	if (size < temp)
 		temp = size;
 	size -= temp;
 	next += temp;

Quoted 4 lines

... although if that's the case, I'd think that the *snprintf() signatures are incorrect: they should return "unsigned" not "int". If that were done, I think even GCC could be made to report such issues; one wouldn't need less-common tools like coccinelle.

We can take it up with ANSI, but I'm not sure they'll be interested in changing the return type of snprintf ...