Subject: Re: CVS commit: syssrc/sys/kern
To: Chris Gilbert <chris@dokein.co.uk>
From: Dave Sainty <dave@dtsp.co.nz>
List: source-changes
Date: 10/03/2002 23:48:29
Chris Gilbert writes:
> 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)
Or if the code doesn't say so itself, at least a comment stating the
implied negative test happens there and is the intention...
With a cast it may be pretty clear what the code does, but it isn't
necessarily clear what the programmers intention was...