Re: CVS commit: syssrc/sys/kern

[Chris Gilbert <chris%dokein.co.uk@localhost>]

On Wed, 02 Oct 2002 22:14:37 -0700
Matt Thomas <matt%3am-software.com@localhost> wrote:

> At 10:06 PM 10/2/2002, itojun%iijlab.net@localhost wrote:
> > >> Modified Files:
> > >>         syssrc/sys/kern: kern_resource.c
> > >>
> > >> Log Message:
> > >> check negative arg.  from openbsd
> > >Due to the `(u_int)' cast -- which I added just about 5 years ago --
> > >this code already handled negative arguments correctly.  Your change
> > >is a noop.
> >
> >         ok, but isn't it better to explicitly check
> >                 if (which < 0 || which >= MAX)
> >                         return EINVAL
> >         than
> >                 if ((u_int)which >= MAX)
> >                         return EINVAL
> >         from readability/clarity?
> 
> No.  It's slower and not if you don't know about signedness/unsigness
> of number, you shouldn't be doing kernel programming.

It's not slower (not unless it's got some magic quantum element hidden in it 8) 
as the compiler optimises it to the same thing.  I'd prefer readabilty over 
casting because some thinks it's faster. (note I'll happily admit 5 years back 
it probably was faster)

Certainly the arm compiler does:
#define MAX 4098
if (which < 0 || which >= MAX)
    return EINVAL
        mov     r3, #4096
        add     r3, r3, #1
        cmp     r0, r3
        movhi   r0, #22
        movls   r0, #0

if ((u_int)which >= MAX)
    return EINVAL;
        mov     r3, #4096
        add     r3, r3, #1
        cmp     r0, r3
        movhi   r0, #22
        movls   r0, #0

(note that i386 gcc produces identical code blocks, but I don't understand i386 
asm that well, so not shown 8)

Chris