[PATCH v3 0/2] Support dates outside of 1901..2038 range

[PATCH v3 0/2] Support dates outside of 1901..2038 range

[Andrew Hamilton]

Support dates outside of 1901..2038.
Add tests for dates outside this range.
Vast majority of the work was done by Vladimir Serbinenko

Fixes: https://savannah.gnu.org/bugs/?63894
Fixes: https://savannah.gnu.org/bugs/?66301

V2 -> V3: Added additional commentary to datetime.c to describe
 the origin of some of the numbers used in calculations.

Andrew Hamilton (2):
  datetime: Support dates outside of 1901..2038 range
  date_unit_test: test dates outside of 32-bit unix range

 grub-core/lib/datetime.c | 48 ++++++++++++++++++++++++++++++++--------
 include/grub/datetime.h  | 15 ++++++-------
 tests/date_unit_test.c   | 35 +++++++++++++++++++++--------
 3 files changed, 72 insertions(+), 26 deletions(-)

-- 
2.39.5


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v3 1/2] datetime: Support dates outside of 1901..2038 range

[Andrew Hamilton]

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 <phcoder@gmail.com>
Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
---
 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) */
+  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 */
+  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);
   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;
+
   ret += monthssum[datetime->month - 1] * SECPERDAY;
-  if (ay == 3 && datetime->month >= 3)
+  int isbisextile = ay == 3 && (datetime->year % 100 != 0 || datetime->year % 400 == 0);
+  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))
       || datetime->day < 1)
     return 0;
 
-- 
2.39.5


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

[PATCH v3 2/2] date_unit_test: test dates outside of 32-bit unix range

[Andrew Hamilton]

Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
---
 tests/date_unit_test.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/tests/date_unit_test.c b/tests/date_unit_test.c
index 99774f199..c76f37cb5 100644
--- a/tests/date_unit_test.c
+++ b/tests/date_unit_test.c
@@ -25,12 +25,13 @@
 #include <grub/test.h>
 
 static void
-date_test (grub_int32_t v)
+date_test (grub_int64_t v)
 {
   struct grub_datetime dt;
   time_t t = v;
   struct tm *g;
   int w;
+  grub_int64_t back = 0;
 
   g = gmtime (&t);
 
@@ -38,28 +39,33 @@ date_test (grub_int32_t v)
 
   w = grub_get_weekday (&dt);
 
-  grub_test_assert (g->tm_sec == dt.second, "time %d bad second: %d vs %d", v,
+  grub_datetime2unixtime (&dt, &back);
+
+  grub_test_assert (g->tm_sec == dt.second, "time %lld bad second: %d vs %d", (long long)v,
 		    g->tm_sec, dt.second);
-  grub_test_assert (g->tm_min == dt.minute, "time %d bad minute: %d vs %d", v,
+  grub_test_assert (g->tm_min == dt.minute, "time %lld bad minute: %d vs %d", (long long)v,
 		    g->tm_min, dt.minute);
-  grub_test_assert (g->tm_hour == dt.hour, "time %d bad hour: %d vs %d", v,
+  grub_test_assert (g->tm_hour == dt.hour, "time %lld bad hour: %d vs %d", (long long)v,
 		    g->tm_hour, dt.hour);
-  grub_test_assert (g->tm_mday == dt.day, "time %d bad day: %d vs %d", v,
+  grub_test_assert (g->tm_mday == dt.day, "time %lld bad day: %d vs %d", (long long)v,
 		    g->tm_mday, dt.day);
-  grub_test_assert (g->tm_mon + 1 == dt.month, "time %d bad month: %d vs %d", v,
+  grub_test_assert (g->tm_mon + 1 == dt.month, "time %lld bad month: %d vs %d",(long long)v,
 		    g->tm_mon + 1, dt.month);
   grub_test_assert (g->tm_year + 1900 == dt.year,
-		    "time %d bad year: %d vs %d", v,
+                   "time %lld bad year: %d vs %d", (long long)v,
 		    g->tm_year + 1900, dt.year);
-  grub_test_assert (g->tm_wday == w, "time %d bad week day: %d vs %d", v,
+  grub_test_assert (g->tm_wday == w, "time %lld bad week day: %d vs %d", (long long)v,
 		    g->tm_wday, w);
+  grub_test_assert (back == v, "time %lld bad back transform: %lld", (long long)v,
+                   (long long)back);
 }
 
 static void
 date_test_iter (void)
 {
-  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
+  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
 			   GRUB_INT32_MAX };
+  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };
   unsigned i;
 
   for (i = 0; i < ARRAY_SIZE (tests); i++)
@@ -71,6 +77,17 @@ date_test_iter (void)
       date_test (x);
       date_test (-x);
     }
+  if (sizeof (time_t) > 4)
+    {
+      for (i = 0; i < ARRAY_SIZE (tests64); i++)
+        date_test (tests64[i]);
+      for (i = 0; i < 10000000; i++)
+        {
+          /* Ranges from 0064 to 6869.  */
+          grub_int64_t x = rand () + (rand () % 100 - 28) * (grub_uint64_t)GRUB_INT32_MAX;
+          date_test (x);
+        }
+    }
 }
 
 GRUB_UNIT_TEST ("date_unit_test", date_test_iter);
-- 
2.39.5


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 0/2] Support dates outside of 1901..2038 range

[Andrew Hamilton]

[-- Attachment #1.1: Type: text/plain, Size: 947 bytes --]

Hello,

I was wondering if this was still on the radar.

Thanks,
Andrew

On Fri, Apr 18, 2025 at 9:54 AM Andrew Hamilton <adhamilt@gmail.com> wrote:

> Support dates outside of 1901..2038.
> Add tests for dates outside this range.
> Vast majority of the work was done by Vladimir Serbinenko
>
> Fixes: https://savannah.gnu.org/bugs/?63894
> Fixes: https://savannah.gnu.org/bugs/?66301
>
> V2 -> V3: Added additional commentary to datetime.c to describe
>  the origin of some of the numbers used in calculations.
>
> Andrew Hamilton (2):
>   datetime: Support dates outside of 1901..2038 range
>   date_unit_test: test dates outside of 32-bit unix range
>
>  grub-core/lib/datetime.c | 48 ++++++++++++++++++++++++++++++++--------
>  include/grub/datetime.h  | 15 ++++++-------
>  tests/date_unit_test.c   | 35 +++++++++++++++++++++--------
>  3 files changed, 72 insertions(+), 26 deletions(-)
>
> --
> 2.39.5
>
>

[-- Attachment #1.2: Type: text/html, Size: 1573 bytes --]

[-- Attachment #2: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 0/2] Support dates outside of 1901..2038 range

[Daniel Kiper]

On Fri, May 23, 2025 at 07:23:32AM -0500, Andrew Hamilton wrote:
> Hello,
>
> I was wondering if this was still on the radar.

Yes, it is but it is not the highest priority...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 1/2] datetime: Support dates outside of 1901..2038 range

[Daniel Kiper via Grub-devel]

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 <phcoder@gmail.com>
> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> ---
>  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
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/2] date_unit_test: test dates outside of 32-bit unix range

[Daniel Kiper via Grub-devel]

On Fri, Apr 18, 2025 at 09:54:00AM -0500, Andrew Hamilton wrote:
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> ---
>  tests/date_unit_test.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/tests/date_unit_test.c b/tests/date_unit_test.c
> index 99774f199..c76f37cb5 100644
> --- a/tests/date_unit_test.c
> +++ b/tests/date_unit_test.c
> @@ -25,12 +25,13 @@
>  #include <grub/test.h>
>
>  static void
> -date_test (grub_int32_t v)
> +date_test (grub_int64_t v)
>  {
>    struct grub_datetime dt;
>    time_t t = v;
>    struct tm *g;
>    int w;
> +  grub_int64_t back = 0;
>
>    g = gmtime (&t);
>
> @@ -38,28 +39,33 @@ date_test (grub_int32_t v)
>
>    w = grub_get_weekday (&dt);
>
> -  grub_test_assert (g->tm_sec == dt.second, "time %d bad second: %d vs %d", v,
> +  grub_datetime2unixtime (&dt, &back);
> +
> +  grub_test_assert (g->tm_sec == dt.second, "time %lld bad second: %d vs %d", (long long)v,

Missing space before "v"...

>  		    g->tm_sec, dt.second);
> -  grub_test_assert (g->tm_min == dt.minute, "time %d bad minute: %d vs %d", v,
> +  grub_test_assert (g->tm_min == dt.minute, "time %lld bad minute: %d vs %d", (long long)v,

Ditto... Here and below...

>  		    g->tm_min, dt.minute);
> -  grub_test_assert (g->tm_hour == dt.hour, "time %d bad hour: %d vs %d", v,
> +  grub_test_assert (g->tm_hour == dt.hour, "time %lld bad hour: %d vs %d", (long long)v,
>  		    g->tm_hour, dt.hour);
> -  grub_test_assert (g->tm_mday == dt.day, "time %d bad day: %d vs %d", v,
> +  grub_test_assert (g->tm_mday == dt.day, "time %lld bad day: %d vs %d", (long long)v,
>  		    g->tm_mday, dt.day);
> -  grub_test_assert (g->tm_mon + 1 == dt.month, "time %d bad month: %d vs %d", v,
> +  grub_test_assert (g->tm_mon + 1 == dt.month, "time %lld bad month: %d vs %d",(long long)v,
>  		    g->tm_mon + 1, dt.month);
>    grub_test_assert (g->tm_year + 1900 == dt.year,
> -		    "time %d bad year: %d vs %d", v,
> +                   "time %lld bad year: %d vs %d", (long long)v,
>  		    g->tm_year + 1900, dt.year);
> -  grub_test_assert (g->tm_wday == w, "time %d bad week day: %d vs %d", v,
> +  grub_test_assert (g->tm_wday == w, "time %lld bad week day: %d vs %d", (long long)v,
>  		    g->tm_wday, w);
> +  grub_test_assert (back == v, "time %lld bad back transform: %lld", (long long)v,
> +                   (long long)back);
>  }
>
>  static void
>  date_test_iter (void)
>  {
> -  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
> +  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
>  			   GRUB_INT32_MAX };
> +  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };

It would be nice to know how these values were chosen...

>    unsigned i;
>
>    for (i = 0; i < ARRAY_SIZE (tests); i++)
> @@ -71,6 +77,17 @@ date_test_iter (void)
>        date_test (x);
>        date_test (-x);
>      }
> +  if (sizeof (time_t) > 4)
> +    {
> +      for (i = 0; i < ARRAY_SIZE (tests64); i++)
> +        date_test (tests64[i]);
> +      for (i = 0; i < 10000000; i++)
> +        {
> +          /* Ranges from 0064 to 6869.  */

How were you come up with this range?

> +          grub_int64_t x = rand () + (rand () % 100 - 28) * (grub_uint64_t)GRUB_INT32_MAX;

Where these numbers come from?

... and missing space before GRUB_INT32_MAX...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/2] date_unit_test: test dates outside of 32-bit unix range

[Daniel Kiper via Grub-devel]

On Fri, Apr 18, 2025 at 09:54:00AM -0500, Andrew Hamilton wrote:
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
>  static void

[...]

>  date_test_iter (void)
>  {
> -  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
> +  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
>  			   GRUB_INT32_MAX };
> +  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };

... and I think it would be more correct to cast these values to
grub_int64_t instead of using "LL"...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 1/2] datetime: Support dates outside of 1901..2038 range

[Andrew Hamilton]

> 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 <phcoder@gmail.com>
> > Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> > ---
> >  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 <daniel.kiper@oracle.com> 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 <phcoder@gmail.com>
> > Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> > ---
> >  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
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH v3 2/2] date_unit_test: test dates outside of 32-bit unix range

[Andrew Hamilton]

> > -  grub_test_assert (g->tm_sec == dt.second, "time %d bad second: %d vs %d", v,
> > +  grub_datetime2unixtime (&dt, &back);
> > +
> > +  grub_test_assert (g->tm_sec == dt.second, "time %lld bad second: %d vs %d", (long long)v,
>
> Missing space before "v"...
Agreed, will fix in v4.

> >                   g->tm_sec, dt.second);
> > -  grub_test_assert (g->tm_min == dt.minute, "time %d bad minute: %d vs %d", v,
> > +  grub_test_assert (g->tm_min == dt.minute, "time %lld bad minute: %d vs %d", (long long)v,
>
> Ditto... Here and below...
Agreed, will fix in v4.

> > -  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
> > +  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
> >                          GRUB_INT32_MAX };
> > +  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };
>
> It would be nice to know how these values were chosen...
Agreed, I will investigate and add appropriate commentary in v4.

> > +      for (i = 0; i < ARRAY_SIZE (tests64); i++)
> > +        date_test (tests64[i]);
> > +      for (i = 0; i < 10000000; i++)
> > +        {
> > +          /* Ranges from 0064 to 6869.  */
>
> How were you come up with this range?
I will investigate and add appropriate commentary in v4.

> >
> > > +          grub_int64_t x = rand () + (rand () % 100 - 28) * (grub_uint64_t)GRUB_INT32_MAX;
> >
> Where these numbers come from?
I will investigate and add appropriate commentary in v4.

>
> ... and missing space before GRUB_INT32_MAX...
Agreed, will fix in V4.

> >  static void
> >  date_test_iter (void)
> >  {
> > -  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
> > +  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
> >                          GRUB_INT32_MAX };
> > +  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };
>
> ... and I think it would be more correct to cast these values to
> grub_int64_t instead of using "LL"...
Ok, I will make this change in v4.

Thanks,
Andrew


On Mon, Aug 25, 2025 at 12:33 PM Daniel Kiper <daniel.kiper@oracle.com> wrote:
>
> On Fri, Apr 18, 2025 at 09:54:00AM -0500, Andrew Hamilton wrote:
> > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> > Signed-off-by: Andrew Hamilton <adhamilt@gmail.com>
> >  static void
>
> [...]
>
> >  date_test_iter (void)
> >  {
> > -  grub_int32_t tests[] = { -1, 0, +1, -2133156255, GRUB_INT32_MIN,
> > +  grub_int32_t tests[] = { -1, 0, +1, 978224552, -2133156255, -2110094321, GRUB_INT32_MIN,
> >                          GRUB_INT32_MAX };
> > +  grub_int64_t tests64[] = { 5774965200LL, 4108700725LL, -5029179792LL };
>
> ... and I think it would be more correct to cast these values to
> grub_int64_t instead of using "LL"...
>
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel