All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
@ 2024-08-17 17:30 Vladimir Serbinenko
  2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Vladimir Serbinenko @ 2024-08-17 17:30 UTC (permalink / raw)
  To: grub-devel; +Cc: Vladimir Serbinenko

Signed-off-by: Vladimir Serbinenko <phcoder@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;
 
-  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 */
+  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 */
@@ -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);
   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.2


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range
  2024-08-17 17:30 [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Vladimir Serbinenko
@ 2024-08-17 17:30 ` Vladimir Serbinenko
  2024-08-18  1:40   ` Andrew Hamilton
  2024-09-02 16:26   ` Daniel Kiper
  2024-08-18  2:06 ` [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Andrew Hamilton
  2024-09-02 16:13 ` Daniel Kiper
  2 siblings, 2 replies; 9+ messages in thread
From: Vladimir Serbinenko @ 2024-08-17 17:30 UTC (permalink / raw)
  To: grub-devel; +Cc: Vladimir Serbinenko

Signed-off-by: Vladimir Serbinenko <phcoder@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..a489fc3d8 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.2


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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range
  2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
@ 2024-08-18  1:40   ` Andrew Hamilton
  2024-09-02 16:26   ` Daniel Kiper
  1 sibling, 0 replies; 9+ messages in thread
From: Andrew Hamilton @ 2024-08-18  1:40 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Vladimir Serbinenko

Thank you for doing this, it worked perfectly for me when I tested it
on x86-64 in a VM.

Sincerely,
Andrew

On Sat, Aug 17, 2024 at 12:31 PM Vladimir Serbinenko <phcoder@gmail.com> wrote:
>
> Signed-off-by: Vladimir Serbinenko <phcoder@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..a489fc3d8 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.2
>
>
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
  2024-08-17 17:30 [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Vladimir Serbinenko
  2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
@ 2024-08-18  2:06 ` Andrew Hamilton
  2024-09-02 16:13 ` Daniel Kiper
  2 siblings, 0 replies; 9+ messages in thread
From: Andrew Hamilton @ 2024-08-18  2:06 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Vladimir Serbinenko

Thank you for doing this, it worked perfectly for me when I tested it
on x86-64 in a VM.

Testing performed:
1. Set various files on FAT32 and Ext4 file systems with various dates
in the future (2038, 2040, 2100, etc.) and confirmed correct date /
time display via GRUB 'ls' command.
2. Ran your provided updated unit test suite
3. Confirmed booting of a Linux OS in a VM with the updated GRUB worked normally
4. Ran the updated 'grub_datetime2unixtime' function for all years
[1900 to 2999] / months [0 to 13] / days [0 to 32] with combinations
of random in / out of bounds hours/minutes/seconds - confirmed
identical results to an alternate implementation. Also ran this with
various compiler sanitizers enabled - no issues seen.
5. Ran the updated 'grub_datetime2unixtime' function with maximum
(65535) and minimum (1) valid year / month / day / hour / min / second
- no issues seen.
6. I ran coverage on the updated 'grub_datetime2unixtime' and was able
to cover all statements except the overflow check at the end (not
saying this has to be taken out - maybe be a good sanity check):
if ((datetime->year > 1980 && ret < 0)
   339003:  155:      || (datetime->year < 1960 && ret > 0))
    #####:  156:    return 0;


Sincerely,
Andrew

On Sat, Aug 17, 2024 at 12:31 PM Vladimir Serbinenko <phcoder@gmail.com> wrote:
>
> Signed-off-by: Vladimir Serbinenko <phcoder@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;
>
> -  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 */
> +  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 */
> @@ -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);
>    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.2
>
>
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
  2024-08-17 17:30 [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Vladimir Serbinenko
  2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
  2024-08-18  2:06 ` [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Andrew Hamilton
@ 2024-09-02 16:13 ` Daniel Kiper
  2024-10-26 15:00   ` Andrew Hamilton
  2 siblings, 1 reply; 9+ messages in thread
From: Daniel Kiper @ 2024-09-02 16:13 UTC (permalink / raw)
  To: Vladimir Serbinenko; +Cc: grub-devel

On Sat, Aug 17, 2024 at 08:30:22PM +0300, Vladimir Serbinenko wrote:
> Signed-off-by: Vladimir Serbinenko <phcoder@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range
  2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
  2024-08-18  1:40   ` Andrew Hamilton
@ 2024-09-02 16:26   ` Daniel Kiper
  1 sibling, 0 replies; 9+ messages in thread
From: Daniel Kiper @ 2024-09-02 16:26 UTC (permalink / raw)
  To: Vladimir Serbinenko; +Cc: grub-devel

On Sat, Aug 17, 2024 at 08:30:23PM +0300, Vladimir Serbinenko wrote:
> Signed-off-by: Vladimir Serbinenko <phcoder@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..a489fc3d8 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 };

Where do these numbers come from?

>    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.  */

Why?

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

Nit, wrong coding style for cast. I can see similar problems in other
places too...

Daniel

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
  2024-09-02 16:13 ` Daniel Kiper
@ 2024-10-26 15:00   ` Andrew Hamilton
  2024-12-19 17:34     ` Andrew Hamilton
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Hamilton @ 2024-10-26 15:00 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Vladimir Serbinenko


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

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 <dkiper@net-space.pl> wrote:

> On Sat, Aug 17, 2024 at 08:30:22PM +0300, Vladimir Serbinenko wrote:
> > Signed-off-by: Vladimir Serbinenko <phcoder@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
>

[-- Attachment #1.2: Type: text/html, Size: 8113 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
  2024-10-26 15:00   ` Andrew Hamilton
@ 2024-12-19 17:34     ` Andrew Hamilton
  2025-04-17 21:31       ` Andrew Hamilton
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Hamilton @ 2024-12-19 17:34 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Vladimir Serbinenko, Daniel Kiper


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

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 <adhamilt@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 <dkiper@net-space.pl> wrote:
>
>> On Sat, Aug 17, 2024 at 08:30:22PM +0300, Vladimir Serbinenko wrote:
>> > Signed-off-by: Vladimir Serbinenko <phcoder@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
>>
>

[-- Attachment #1.2: Type: text/html, Size: 8653 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range
  2024-12-19 17:34     ` Andrew Hamilton
@ 2025-04-17 21:31       ` Andrew Hamilton
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Hamilton @ 2025-04-17 21:31 UTC (permalink / raw)
  To: The development of GNU GRUB; +Cc: Vladimir Serbinenko, Daniel Kiper


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

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 <adhamilt@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 <adhamilt@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 <dkiper@net-space.pl> wrote:
>>
>>> On Sat, Aug 17, 2024 at 08:30:22PM +0300, Vladimir Serbinenko wrote:
>>> > Signed-off-by: Vladimir Serbinenko <phcoder@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
>>>
>>

[-- Attachment #1.2: Type: text/html, Size: 9253 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-04-17 21:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-17 17:30 [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Vladimir Serbinenko
2024-08-17 17:30 ` [PATCH v2 2/2] date_unit_test: test dates outside of 32-bit unix range Vladimir Serbinenko
2024-08-18  1:40   ` Andrew Hamilton
2024-09-02 16:26   ` Daniel Kiper
2024-08-18  2:06 ` [PATCH v2 1/2] datetime: Support dates outside of 1901..2038 range Andrew Hamilton
2024-09-02 16:13 ` Daniel Kiper
2024-10-26 15:00   ` Andrew Hamilton
2024-12-19 17:34     ` Andrew Hamilton
2025-04-17 21:31       ` Andrew Hamilton

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.