> On Fri, Apr 18, 2025 at 09:53:59AM -0500, Andrew Hamilton wrote:
> > Support dates outside of 1901..2038.
> >
> > Fixes: https://savannah.gnu.org/bugs/?63894
> > Fixes: https://savannah.gnu.org/bugs/?66301
> >
> > Signed-off-by: Vladimir Serbinenko <[email protected]>
> > Signed-off-by: Andrew Hamilton <[email protected]>
> > ---
> > grub-core/lib/datetime.c | 48 ++++++++++++++++++++++++++++++++--------
> > include/grub/datetime.h | 15 ++++++-------
> > 2 files changed, 46 insertions(+), 17 deletions(-)
> >
> > diff --git a/grub-core/lib/datetime.c b/grub-core/lib/datetime.c
> > index 8f0922fb0..4e68eabc6 100644
> > --- a/grub-core/lib/datetime.c
> > +++ b/grub-core/lib/datetime.c
> > @@ -64,6 +64,10 @@ grub_get_weekday_name (struct grub_datetime *datetime)
> > #define SECPERDAY (24*SECPERHOUR)
> > #define DAYSPERYEAR 365
> > #define DAYSPER4YEARS (4*DAYSPERYEAR+1)
> > +/* 24 leap years in 100 years */
> > +#define DAYSPER100YEARS (100*DAYSPERYEAR+24)
> > +/* 97 leap years in 400 years */
> > +#define DAYSPER400YEARS (400*DAYSPERYEAR+97)
> >
> >
> > void
> > @@ -76,7 +80,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;
> > @@ -92,12 +96,39 @@ 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;
> > + /* 1970 is Unix Epoch. Adjust to a year 1 epoch:
> > + Leap year logic:
> > + - Years evenly divisible by 400 are leap years
> > + - Otherwise, if divisible by 100 are not leap years
> > + - Otherwise, if divisible by 4 are leap years
> > + There are four 400-year periods (1600 years worth of days with leap
> > days)
> > + There are three 100-year periods worth of leap days (3*24)
> > + There are 369 years in addition to the four 400 year periods
> > + There are 17 leap days in 69 years (beyond the three 100 year
> > periods) */
>
> May I ask you to stick to comments coding style [1]?
>
> > + days = days_epoch + 369 * DAYSPERYEAR + 17 + 24 * 3 + 4 *
> > DAYSPER400YEARS;
> >
> > - datetime->year = 1901 + 4 * (days / DAYSPER4YEARS);
> > + datetime->year = 1 + 400 * (days / DAYSPER400YEARS);
> > + days %= DAYSPER400YEARS;
> > +
> > + /* On 31st December of bissextile (leap) years 365 days from the
> > beginning
> > + of the year elapsed but year isn't finished yet - every 400 years
> > + 396 is 4 years less than 400 year leap cycle
> > + 96 is 1 day less than number of leap days in 400 years */
>
> Ditto... Here and below please...
Will do, I will fix the comment formatting in the next version.
> > + 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 */
> > + /* On 31st December of bissextile (leap) years 365 days from the
> > beginning
> > + of the year elapsed but year isn't finished yet - every 4 years */
> > if (days / DAYSPERYEAR == 4)
> > {
> > datetime->year += 3;
> > @@ -108,11 +139,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);
>
> Please do not define variables in random places. I prefer if it is done
> at the beginning of the function or block.
>
> And I think this should be a bool set with ternary operator...
>
> ... and s/isbisextile/is_bisextile/
>
I will correct the variable declaration location and change to bool /
changing naming as well in the next version.
> > 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]);
>
> If you touch the code then please fix its coding style, e.g.:
>
> days -= (i == 1 && is_bisextile == true) ? 29 : months[i]
>
> On the occasion it seems more readable...
I will correct this in the next version.
>
> > 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;
>
> What does this line do? And where 15 comes from?
I will investigate the origin / meaning of "15" here and add comments
to explain this in the next version.
Thank you for the comments, I will try to get a V4 out tonight.
Thanks!
Andrew
On Mon, Aug 25, 2025 at 12:08 PM Daniel Kiper <[email protected]> wrote:
>
> On Fri, Apr 18, 2025 at 09:53:59AM -0500, Andrew Hamilton wrote:
> > Support dates outside of 1901..2038.
> >
> > Fixes: https://savannah.gnu.org/bugs/?63894
> > Fixes: https://savannah.gnu.org/bugs/?66301
> >
> > Signed-off-by: Vladimir Serbinenko <[email protected]>
> > Signed-off-by: Andrew Hamilton <[email protected]>
> > ---
> > grub-core/lib/datetime.c | 48 ++++++++++++++++++++++++++++++++--------
> > include/grub/datetime.h | 15 ++++++-------
> > 2 files changed, 46 insertions(+), 17 deletions(-)
> >
> > diff --git a/grub-core/lib/datetime.c b/grub-core/lib/datetime.c
> > index 8f0922fb0..4e68eabc6 100644
> > --- a/grub-core/lib/datetime.c
> > +++ b/grub-core/lib/datetime.c
> > @@ -64,6 +64,10 @@ grub_get_weekday_name (struct grub_datetime *datetime)
> > #define SECPERDAY (24*SECPERHOUR)
> > #define DAYSPERYEAR 365
> > #define DAYSPER4YEARS (4*DAYSPERYEAR+1)
> > +/* 24 leap years in 100 years */
> > +#define DAYSPER100YEARS (100*DAYSPERYEAR+24)
> > +/* 97 leap years in 400 years */
> > +#define DAYSPER400YEARS (400*DAYSPERYEAR+97)
> >
> >
> > void
> > @@ -76,7 +80,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;
> > @@ -92,12 +96,39 @@ 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;
> > + /* 1970 is Unix Epoch. Adjust to a year 1 epoch:
> > + Leap year logic:
> > + - Years evenly divisible by 400 are leap years
> > + - Otherwise, if divisible by 100 are not leap years
> > + - Otherwise, if divisible by 4 are leap years
> > + There are four 400-year periods (1600 years worth of days with leap
> > days)
> > + There are three 100-year periods worth of leap days (3*24)
> > + There are 369 years in addition to the four 400 year periods
> > + There are 17 leap days in 69 years (beyond the three 100 year
> > periods) */
>
> May I ask you to stick to comments coding style [1]?
>
> > + days = days_epoch + 369 * DAYSPERYEAR + 17 + 24 * 3 + 4 *
> > DAYSPER400YEARS;
> >
> > - datetime->year = 1901 + 4 * (days / DAYSPER4YEARS);
> > + datetime->year = 1 + 400 * (days / DAYSPER400YEARS);
> > + days %= DAYSPER400YEARS;
> > +
> > + /* On 31st December of bissextile (leap) years 365 days from the
> > beginning
> > + of the year elapsed but year isn't finished yet - every 400 years
> > + 396 is 4 years less than 400 year leap cycle
> > + 96 is 1 day less than number of leap days in 400 years */
>
> Ditto... Here and below please...
>
> > + if (days / DAYSPER100YEARS == 4)
> > + {
> > + datetime->year += 396;
> > + days -= 396*DAYSPERYEAR + 96;
> > + }
> > + else
> > + {
> > + datetime->year += 100 * (days / DAYSPER100YEARS);
> > + days %= DAYSPER100YEARS;
> > + }
> > +
> > + 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 */
> > + /* On 31st December of bissextile (leap) years 365 days from the
> > beginning
> > + of the year elapsed but year isn't finished yet - every 4 years */
> > if (days / DAYSPERYEAR == 4)
> > {
> > datetime->year += 3;
> > @@ -108,11 +139,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);
>
> Please do not define variables in random places. I prefer if it is done
> at the beginning of the function or block.
>
> And I think this should be a bool set with ternary operator...
>
> ... and s/isbisextile/is_bisextile/
>
> > 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]);
>
> If you touch the code then please fix its coding style, e.g.:
>
> days -= (i == 1 && is_bisextile == true) ? 29 : months[i]
>
> On the occasion it seems more readable...
>
> > 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;
>
> What does this line do? And where 15 comes from?
>
> Daniel
_______________________________________________
Grub-devel mailing list
[email protected]
https://lists.gnu.org/mailman/listinfo/grub-devel
