Subject: Re: CVS commit: syssrc/sys/kern
To: Matt Thomas <matt@3am-software.com>
From: Chris Gilbert <chris@dokein.co.uk>
List: source-changes
Date: 10/03/2002 09:10:04
On Wed, 02 Oct 2002 22:14:37 -0700
Matt Thomas <matt@3am-software.com> wrote:

> At 10:06 PM 10/2/2002, itojun@iijlab.net 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