Subject: Re: CVS commit: src/sys/lib/libkern
To: David Laight <david@l8s.co.uk>
From: Christos Zoulas <christos@zoulas.com>
List: source-changes
Date: 03/28/2006 18:30:39
On Mar 29, 12:08am, david@l8s.co.uk (David Laight) wrote:
-- Subject: Re: CVS commit: src/sys/lib/libkern

| On Tue, Mar 28, 2006 at 06:07:18PM +0000, Christos Zoulas wrote:
| > >
| > >How about casting the result to 'int' ?
| > >The domain of the result will always fit in an int, whereas sizeof
| > >will return a 64bit type on LP64 systems.
| > >Using 'int' (not 'unsigned int') allows:
| > >	int i;
| > >	...
| > >	for (i = 0; i < __arraycount(xxx); i++)
| > >		...
| > >work (without giving a 'signed v unsigned comparison' warning).
| > >
| > 
| > This is wrong given that the number is unsigned. Although it fits in
| > an int, it does not make it signed. You should be converting your
| > loops to use u_int or size_t instead.
| 
| But the domain of the value will fit in an 'int' (we aren't going to
| have arrays with > 2^31 elements, nor a port where int is less than
| 32 bits).
| 
| Having __arraycount() return int is much less likely to introduce bugs - eg
| those caused by comparisons of the loop control variable for < 0.
| We also just don't want the bloat that would happen if the loop control
| variable becomes a 64bit variable.

Indexes in modern code should be unsigned. It is always simpler to check

	u_int me;

	if (me < MAXME)

As opposed to:

	int me;

	if (me >= 0 && me < MAXME)

We've avoided a whole bunch of bugs in the kernel by using unsigned variables.
Let's not undo the good work.

christos
christos