Re: [kvm-unit-tests PATCH] powerpc: Add tests for RTAS

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-03-03 17:29+0100, Paolo Bonzini:
> On 03/03/2016 17:03, Radim Krčmář wrote:
>>> > diff --git a/powerpc/rtas.c b/powerpc/rtas.c
>>> > +#define DAYS(y,m,d) (365UL * (y) + ((y) / 4) - ((y) / 100) + ((y) / 400) + \
>>> > +		     367UL * (m) / 12  + \
>>> > +		     (d))
>> This function is hard to (re)use.
>> What about putting the "month -= 2" block together with DAYS to give a
>> better estimate of the amount of days in the gregorian calendar?
> 
> Even the Gregorian calendar only starts in 1583 though. :)
> 
> This is just a utility function for mktime.  I think it's okay.  We
> should aim at making libcflat a minimal libc, and in that case we would
> move mktime to lib/.  Putting stuff directly in tests is good enough
> (worse is better), but let's remember that duplicated code is not.

I agree that there is no need to move stuff into lib/.
I just wouldn't expose DAYS in this shape even to mktime, because DAYS
makes little sense without the "month -= 2" fixup.

>>> > +	/* Put February at end of the year to avoid leap day this year */

Leap day is not the only reason.
'367UL * (m) / 12' equation needs to have shifted months to report the
correct numbers of days throughout any year.

>>> > +	/* compute epoch: substract DAYS(since_March(1-1-1970)) */
>>> > +	epoch = DAYS(year, month, day) - DAYS(1969, 11, 1);

And this comment points out that the API is bad. :)

(Btw. am I going overboard?)