Re: lib/50009: strptime small enhancement

[Brian Ginsbach <ginsbach%netbsd.org@localhost>]

On Mon, Jun 29, 2015 at 02:42:18PM +1000, matthew green wrote:
> 
> >  On Sun, Jun 28, 2015 at 07:50:00PM +0000, devnexen%gmail.com@localhost wrote:
> >   > +#ifndef ISLEAPYEAR
> >   > +#define ISLEAPYEAR(y) ((y % 400) == 0 && (y % 4) == 0 && (y % 100) != 0)
> >   > +#endif
> >  
> >  ...this is wrong...
> 
> indeed, sys/clock.h has this:
> 
> 63 /*
> 64  * This inline avoids some unnecessary modulo operations
> 65  * as compared with the usual macro:
> 66  *   ( ((year % 4) == 0 &&
> 67  *      (year % 100) != 0) ||
> 68  *     ((year % 400) == 0) )
> 69  * It is otherwise equivalent.
> 70  */
> 71 static inline int
> 72 is_leap_year(uint64_t year)
> 73 {
> 74         if ((year & 3) != 0)
> 75                 return 0;
> 76
> 77         if (__predict_false((year % 100) != 0))
> 78                 return 1;
> 79
> 80         return __predict_false((year % 400) == 0);
> 81 }
> 
> which should probably be used instead, for netbsd, or at
> least the same expression in the comment.
> 
> (the macro above also doesn't work properly for various
> inputs, like a good macro should.)
> 
> 

Is there some specific reason why the sys/clock.h version should be preferred over
the macros in libc/time/tzfile.h which are used throughout the rest of the the
libc/time code?

154 #define isleap(y) (((y) % 4) == 0 && (((y) % 100) != 0 || ((y) % 400) == 0))
155 
156 /*
157 ** Since everything in isleap is modulo 400 (or a factor of 400), we know that
158 **      isleap(y) == isleap(y % 400)
159 ** and so
160 **      isleap(a + b) == isleap((a + b) % 400)
161 ** or
162 **      isleap(a + b) == isleap(a % 400 + b % 400)
163 ** This is true even if % means modulo rather than Fortran remainder
164 ** (which is allowed by C89 but not C99).
165 ** We use this to avoid addition overflow problems.
166 */
167 
168 #define isleap_sum(a, b)        isleap((a) % 400 + (b) % 400)

Wouldn't it be more consistent to keep using the same macro(s) throughout all the
libc/time code?

Brian