[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [2.06 RELEASE PATCH 1/3] fs: Use 64bit type for filesystem timestamp
From: |
Daniel Kiper |
Subject: |
Re: [2.06 RELEASE PATCH 1/3] fs: Use 64bit type for filesystem timestamp |
Date: |
Wed, 5 May 2021 16:32:38 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Wed, May 05, 2021 at 10:32:31AM +0200, Javier Martinez Canillas wrote:
> From: Carlos Maiolino <cmaiolino@redhat.com>
>
> Some filesystems nowadays uses 64bit types for timestamps, so, update
s/64bit/64-bit/
> grub_dirhook_info struct to use an int64 type to store mtime. This also
s/int64/grub_int64_t/
> updates grub_unixtime2datetime() to receive a 64-bit timestamp
> argument and do 64bit-safe divisions.
s/64bit-safe/64-bit-safe/
> All the remaining conversion from 32 to 64 should be safe, as 32 to 64
s/32/32-bits/
s/64/64-bits/
...and below please...
> attributions will be implicitly casted. The most crictical part in the
> 32 to 64bits conversion is on grub_unixtime2datetime() where it needs
> to deal with the 64bit type, specially with division in x86_architectures,
I think it is not "x86_architectures" specific. In general it applies to
all architectures which does not provide native 64-bit division. So,
I think this sentence should be rephrased a bit.
> so, for that, the grub_divmod64() helper has been used.
>
> These changes enables grub to support dates beyond y2038.
>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
This line should be dropped.
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
[...]
> diff --git a/grub-core/lib/datetime.c b/grub-core/lib/datetime.c
> index 95b8c9ff5e3..3e84fa1dbc4 100644
> --- a/grub-core/lib/datetime.c
> +++ b/grub-core/lib/datetime.c
> @@ -19,6 +19,7 @@
>
> #include <grub/datetime.h>
> #include <grub/i18n.h>
> +#include <grub/misc.h>
>
> static const char *const grub_weekday_names[] =
> {
> @@ -60,9 +61,10 @@ grub_get_weekday_name (struct grub_datetime *datetime)
>
>
> void
> -grub_unixtime2datetime (grub_int32_t nix, struct grub_datetime *datetime)
> +grub_unixtime2datetime (grub_int64_t nix, struct grub_datetime *datetime)
> {
> int i;
> + grub_uint64_t rem;
If you do not care about reminder please drop it and use NULL instead.
> grub_uint8_t months[12] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
> /* In the period of validity of unixtime all years divisible by 4
> are bissextile*/
> @@ -75,9 +77,16 @@ grub_unixtime2datetime (grub_int32_t nix, struct
> grub_datetime *datetime)
> unsigned secs_in_day;
> /* Transform C divisions and modulos to mathematical ones */
> if (nix < 0)
> - days_epoch = -(((unsigned) (SECPERDAY-nix-1)) / SECPERDAY);
> + /*
> + * the division here shouldn't be larger than INT_MAX,
> + * so, it's safe to store the result back in an int
> + */
> + days_epoch = -(grub_divmod64(((grub_int64_t)(SECPERDAY) - nix - 1),
> + SECPERDAY,
> + &rem));
days_epoch = -(grub_divmod64 (((grub_int64_t) (SECPERDAY) - nix - 1),
SECPERDAY, NULL));
Missing spaces...
> else
> - days_epoch = ((unsigned) nix) / SECPERDAY;
> + days_epoch = grub_divmod64(nix, SECPERDAY, &rem);
days_epoch = grub_divmod64 (nix, SECPERDAY, NULL);
Ditto...
> secs_in_day = nix - days_epoch * SECPERDAY;
> days = days_epoch + 69 * DAYSPERYEAR + 17;
>
> diff --git a/grub-core/net/bootp.c b/grub-core/net/bootp.c
> index e33be51f83b..6fb5627025d 100644
> --- a/grub-core/net/bootp.c
> +++ b/grub-core/net/bootp.c
> @@ -468,7 +468,7 @@ send_dhcp_packet (struct grub_net_network_level_interface
> *iface)
> grub_err_t err;
> struct grub_net_bootp_packet *pack;
> struct grub_datetime date;
> - grub_int32_t t = 0;
> + grub_int64_t t = 0;
> struct grub_net_buff *nb;
> struct udphdr *udph;
> grub_net_network_level_address_t target;
> diff --git a/grub-core/normal/misc.c b/grub-core/normal/misc.c
> index 8bb6da31fb3..f7e9e3ac4a1 100644
> --- a/grub-core/normal/misc.c
> +++ b/grub-core/normal/misc.c
> @@ -136,7 +136,7 @@ grub_normal_print_device_info (const char *name)
> }
> if (fs->fs_mtime)
> {
> - grub_int32_t tm;
> + grub_int64_t tm;
> struct grub_datetime datetime;
> (fs->fs_mtime) (dev, &tm);
> if (grub_errno == GRUB_ERR_NONE)
> diff --git a/grub-core/tests/sleep_test.c b/grub-core/tests/sleep_test.c
> index 3d11c717cb7..63f6713165f 100644
> --- a/grub-core/tests/sleep_test.c
> +++ b/grub-core/tests/sleep_test.c
> @@ -32,7 +32,7 @@ static void
> sleep_test (void)
> {
> struct grub_datetime st, en;
> - grub_int32_t stu = 0, enu = 0;
> + grub_int64_t stu = 0, enu = 0;
> int is_delayok;
> grub_test_assert (!grub_get_datetime (&st), "Couldn't retrieve start
> time");
> grub_millisleep (10000);
> @@ -45,7 +45,7 @@ sleep_test (void)
> if (enu - stu >= 15 && enu - stu <= 17)
> is_delayok = 1;
> #endif
> - grub_test_assert (is_delayok, "Interval out of range: %d", enu-stu);
> + grub_test_assert (is_delayok, "Interval out of range: %lld", enu-stu);
"%lld" is not correct here. I would define PRIdGRUB_INT64_T in
include/grub/types.h and use it here.
...and s/enu-stu/enu - stu/
Daniel