cvs commit: src/sys/i386/isa clock.c

[Bruce Evans]

On Sat, 31 May 2003, Erik Trulsson wrote:

> On Sat, May 31, 2003 at 01:23:46PM -0700, Poul-Henning Kamp wrote:
> > phk         2003/05/31 13:23:46 PDT
> >
> >   FreeBSD src repository
> >
> >   Modified files:
> >     sys/i386/isa         clock.c
> >   Log:
> >   Don't rely on boolean expression evaluating to 1 or 0 by default.
>
> Why not?  In C the relational operators (including '==') *always*
> evaluate to 1 or 0.  One can rely on that.
> I.e. this change doesn't really accomplish anything except make the
> expression somewhat more complicated.

The result of the expression is used as a non-boolean in some places, and
setting its values explicitly is clearer there.  The values are 0 and the
number of extra days in February in a leap year; the latter just happens
to be 1.  In places where the expression is used as a boolean, the change
is an obfuscation.

%%%
$ grep LEAPYEAR clock.c
#define	LEAPYEAR(y) (((u_int)(y) % 4 == 0) ? 1 : 0)
	if ((month > 2) && LEAPYEAR(year))
		days += DAYSPERYEAR + LEAPYEAR(y);
	for (y = 1970, m = DAYSPERYEAR + LEAPYEAR(y);
	     y++,      m = DAYSPERYEAR + LEAPYEAR(y))
		if (m == 1 && LEAPYEAR(y))
%%%

The correct fix here was to replace LEAPYEAR(y) by (LEAPYEAR(y) ? 1 :
0) in the 3 expressions that add it, and not change the boolean
expression LEAPYEAR(y).  LEAPYEAR() is correctly named for a boolean
expression that classifies leap years but bogusly named for the
non-boolean adjustment to the number of days in February, so it shouldn't
have been changed to give the latter as its primary result and the former
as an alias.

Bruce