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