itojun@iijlab.net writes:
> >> >Since *curpps should be unsigned, this would be
> >> >	if (*curpps + 1 != 0)
> >>  	if (*curpps + 1 > *curpps) should do for both signed and unsigned.
> >Again, ommiting this check would be harmless in practice.  Just comparing
> >to zero looks slightly lighter for me (at least on some platforms), though
> >won't affect the total performance at all.
> 
> 	i object.
> 	from my experience, it is bad practice to omit this kind of checks.
> 	it will bite us in the future.  for example, substantial portion of
> 	KAME work was to remove assumptions on mbuf size boundary (which
> 	became false due to like increased struct size), or adding proper
> 	checks/MCLGET calls.

A check is "probably" not ever going to be hit here, but it's good
practice, especially if done cheaply.  The more instructions you
waste, the more annoying the check is, of course, and if it's
expensive enough (i.e. you're actually gonna try the "*curpps + 1 >
*curpps" thing) then it should probably be DIAGNOSTIC.

if you are truly paranoid about this, why not just declare curpps as
you'd like it to be?  make curpps an unsigned int * (i mean, heck, if
it's meant to be unsigned int * then it should be defined as such),
and then:

	if (~(*curpps) != 0)
		...

or (*curpps + 1 != 0) as suggested?  either should work (assuming your
ints are powers of two, and wrap to 0 ... and i sure do hope that's
safe 8-).

(the point here is, the function prototype controls the expected type
of the variable.  you should bloody well feel free to use that to make
the code more efficient.  it's not like there's some deep magic inthis
function.  if you want to change the definition of the variable, then
sure, you might need to change the callers and the implementation of
the function.  that'll always be true.)


cgd