Subject: Re: ppsratecheck(9)
To: Atsushi Onoe <onoe@sm.sony.co.jp>
From: None <itojun@iijlab.net>
List: tech-kern
Date: 07/07/2000 14:54:39
thanks for comments.
>> + int
>> + ppsratecheck(lasttime, curpps, maxpps)
>> + struct timeval *lasttime;
>> + int *curpps;
>> + int maxpps; /* maximum pps allowed */
>*curpps and maxpps should be unsigned.
maybe, maybe not. anyone care about more than 2^31 events?
if really so, long or u_long.
>> + {
>> + s = splclock();
>> + timersub(&mono_time, lasttime, &delta);
>splclock() should only protect reference to mono_time, not whole the
>function.
>> + *lasttime = mono_time;
>mono_time should be cached at the beginning of the function.
i took this part from ratecheck(9).
>> + /*
>> + * check for 0,0 is so that the message will be seen at least once.
>> + * if more than one second have passed since the last update of
>> + * lasttime, start over.
>> + */
>> + if ((lasttime->tv_sec == 0 && lasttime->tv_usec == 0) ||
>> + delta.tv_sec > 0) {
>delta might be negative if administrator reset the date by hand.
>delta.tv_sec != 0 looks more appropriate.
i don't think so.
(1) mono_time will never go back. mono_time(9).
(2) even if it goes back, it is inappropriate to reset *lasttime
when delta.tv_sec < 0.
>> + *curpps = 0;
>> + rv = 1;
>> + } else if (*curpps < maxpps)
>> + rv = 1;
>> + else
>> + rv = 0;
>> + /* be careful about wrap-around */
>> + if (*curpps + 1 > *curpps)
>> + *curpps = *curpps + 1;
>Since *curpps should be unsigned, this would be
> if (*curpps + 1 != 0)
if (*curpps + 1 > *curpps) should do for both signed and unsigned.
itojun