Re: CVS commit: src/sys/lib/libkern

[christos%zoulas.com@localhost (Christos Zoulas)]

On Mar 29, 12:08am, david%l8s.co.uk@localhost (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