Hi Vladimir,
I can make an attempt at addressing Daniel's comments if that's okay - any
objections?
Thanks,
Andrew
On Thu, Dec 19, 2024 at 11:34 AM Andrew Hamilton <adham...@gmail.com> wrote:
> Just checking on this again. I know people are busy so understand.
>
> Thanks,
> Andrew
>
> On Sat, Oct 26, 2024 at 10:00 AM Andrew Hamilton <adham...@gmail.com>
> wrote:
>
>> Just checking if this was still on the radar, seems mostly comment and
>> type changes so maybe a relatively minor update?
>>
>> If any help is needed to make the adjustments let me know and I can help.
>>
>> Thanks,
>> Andrew
>>
>> On Mon, Sep 2, 2024 at 11:14 AM Daniel Kiper <dki...@net-space.pl> wrote:
>>
>>> On Sat, Aug 17, 2024 at 08:30:22PM +0300, Vladimir Serbinenko wrote:
>>> > Signed-off-by: Vladimir Serbinenko <phco...@gmail.com>
>>> > ---
>>> > grub-core/lib/datetime.c | 31 ++++++++++++++++++++++++-------
>>> > include/grub/datetime.h | 15 +++++++--------
>>> > 2 files changed, 31 insertions(+), 15 deletions(-)
>>> >
>>> > diff --git a/grub-core/lib/datetime.c b/grub-core/lib/datetime.c
>>> > index 9120128ca..e4bdc5c4f 100644
>>> > --- a/grub-core/lib/datetime.c
>>> > +++ b/grub-core/lib/datetime.c
>>> > @@ -59,6 +59,8 @@ grub_get_weekday_name (struct grub_datetime
>>> *datetime)
>>> > #define SECPERDAY (24*SECPERHOUR)
>>> > #define DAYSPERYEAR 365
>>> > #define DAYSPER4YEARS (4*DAYSPERYEAR+1)
>>> > +#define DAYSPER100YEARS (100*DAYSPERYEAR+24)
>>> > +#define DAYSPER400YEARS (400*DAYSPERYEAR+97)
>>> >
>>> >
>>> > void
>>> > @@ -71,7 +73,7 @@ grub_unixtime2datetime (grub_int64_t nix, struct
>>> grub_datetime *datetime)
>>> > /* Convenience: let's have 3 consecutive non-bissextile years
>>> > at the beginning of the counting date. So count from 1901. */
>>> > int days_epoch;
>>> > - /* Number of days since 1st Januar, 1901. */
>>> > + /* Number of days since 1st January, 1 (proleptic). */
>>> > unsigned days;
>>> > /* Seconds into current day. */
>>> > unsigned secs_in_day;
>>> > @@ -87,9 +89,25 @@ grub_unixtime2datetime (grub_int64_t nix, struct
>>> grub_datetime *datetime)
>>> > days_epoch = grub_divmod64 (nix, SECPERDAY, NULL);
>>> >
>>> > secs_in_day = nix - days_epoch * SECPERDAY;
>>> > - days = days_epoch + 69 * DAYSPERYEAR + 17;
>>> > + days = days_epoch + 369 * DAYSPERYEAR + 17 + 24 * 3 + 4 *
>>> DAYSPER400YEARS;
>>>
>>> I can imagine how this math was build but I would prefer if you add
>>> a comment where these numbers come from...
>>>
>>> > - datetime->year = 1901 + 4 * (days / DAYSPER4YEARS);
>>> > + datetime->year = 1 + 400 * (days / DAYSPER400YEARS);
>>> > + days %= DAYSPER400YEARS;
>>> > +
>>> > + /* On 31st December of bissextile years 365 days from the beginning
>>> > + of the year elapsed but year isn't finished yet */
>>>
>>> I think the comment is not precise. It happens once per 400 years.
>>> Right? I think it should be clarified.
>>>
>>> > + if (days / DAYSPER100YEARS == 4)
>>> > + {
>>> > + datetime->year += 396;
>>> > + days -= 396*DAYSPERYEAR + 96;
>>>
>>> Where do these numbers come from? The math begs for comment.
>>>
>>> > + }
>>> > + else
>>> > + {
>>> > + datetime->year += 100 * (days / DAYSPER100YEARS);
>>> > + days %= DAYSPER100YEARS;
>>>
>>> Ditto.
>>>
>>> > + }
>>> > +
>>> > + datetime->year += 4 * (days / DAYSPER4YEARS);
>>> > days %= DAYSPER4YEARS;
>>> > /* On 31st December of bissextile years 365 days from the beginning
>>> > of the year elapsed but year isn't finished yet */
>>> > @@ -103,11 +121,10 @@ grub_unixtime2datetime (grub_int64_t nix, struct
>>> grub_datetime *datetime)
>>> > datetime->year += days / DAYSPERYEAR;
>>> > days %= DAYSPERYEAR;
>>> > }
>>> > + int isbisextile = datetime->year % 4 == 0 && (datetime->year % 100
>>> != 0 || datetime->year % 400 == 0);
>>>
>>> Could you use bool instead? And define variable at the beginning of
>>> function?
>>>
>>> > for (i = 0; i < 12
>>> > - && days >= (i==1 && datetime->year % 4 == 0
>>> > - ? 29 : months[i]); i++)
>>> > - days -= (i==1 && datetime->year % 4 == 0
>>> > - ? 29 : months[i]);
>>> > + && days >= (i==1 && isbisextile ? 29 : months[i]); i++)
>>> > + days -= (i==1 && isbisextile ? 29 : months[i]);
>>> > datetime->month = i + 1;
>>> > datetime->day = 1 + days;
>>> > datetime->hour = (secs_in_day / SECPERHOUR);
>>> > diff --git a/include/grub/datetime.h b/include/grub/datetime.h
>>> > index bcec636f0..9289b0d00 100644
>>> > --- a/include/grub/datetime.h
>>> > +++ b/include/grub/datetime.h
>>> > @@ -54,7 +54,7 @@ void grub_unixtime2datetime (grub_int64_t nix,
>>> > static inline int
>>> > grub_datetime2unixtime (const struct grub_datetime *datetime,
>>> grub_int64_t *nix)
>>> > {
>>> > - grub_int32_t ret;
>>> > + grub_int64_t ret;
>>> > int y4, ay;
>>> > const grub_uint16_t monthssum[12]
>>> > = { 0,
>>> > @@ -75,15 +75,11 @@ grub_datetime2unixtime (const struct grub_datetime
>>> *datetime, grub_int64_t *nix)
>>> > const int SECPERHOUR = 60 * SECPERMIN;
>>> > const int SECPERDAY = 24 * SECPERHOUR;
>>> > const int SECPERYEAR = 365 * SECPERDAY;
>>> > - const int SECPER4YEARS = 4 * SECPERYEAR + SECPERDAY;
>>> > + const grub_int64_t SECPER4YEARS = 4 * SECPERYEAR + SECPERDAY;
>>> >
>>> > - if (datetime->year > 2038 || datetime->year < 1901)
>>> > - return 0;
>>> > if (datetime->month > 12 || datetime->month < 1)
>>> > return 0;
>>> >
>>> > - /* In the period of validity of unixtime all years divisible by 4
>>> > - are bissextile*/
>>> > /* Convenience: let's have 3 consecutive non-bissextile years
>>> > at the beginning of the epoch. So count from 1973 instead of
>>> 1970 */
>>> > ret = 3 * SECPERYEAR + SECPERDAY;
>>> > @@ -94,13 +90,16 @@ grub_datetime2unixtime (const struct grub_datetime
>>> *datetime, grub_int64_t *nix)
>>> > ret += y4 * SECPER4YEARS;
>>> > ret += ay * SECPERYEAR;
>>> >
>>> > + ret -= ((datetime->year - 1) / 100 - (datetime->year - 1) / 400 -
>>> 15) * SECPERDAY;
>>>
>>> Again, please comment this math...
>>>
>>> > ret += monthssum[datetime->month - 1] * SECPERDAY;
>>> > - if (ay == 3 && datetime->month >= 3)
>>> > + int isbisextile = ay == 3 && (datetime->year % 100 != 0 ||
>>> datetime->year % 400 == 0);
>>>
>>> Ditto.
>>>
>>> And could you use bool instead of int?
>>>
>>> > + if (isbisextile && datetime->month >= 3)
>>> > ret += SECPERDAY;
>>> >
>>> > ret += (datetime->day - 1) * SECPERDAY;
>>> > if ((datetime->day > months[datetime->month - 1]
>>> > - && (!ay || datetime->month != 2 || datetime->day != 29))
>>> > + && !(isbisextile && datetime->month == 2 && datetime->day ==
>>> 29))
>>>
>>> It would be nice to add a comment here too.
>>>
>>> Daniel
>>>
>>> _______________________________________________
>>> Grub-devel mailing list
>>> Grub-devel@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>>
>>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
