* [PATCH v2 2/2] fat: Support file modification times
@ 2020-03-03 19:41 David Michael
2020-03-06 12:19 ` Daniel Kiper
0 siblings, 1 reply; 4+ messages in thread
From: David Michael @ 2020-03-03 19:41 UTC (permalink / raw)
To: grub-devel
This allows comparing file ages on EFI system partitions.
Signed-off-by: David Michael <fedora.dm0@gmail.com>
---
Changes since v1:
- Added the previous patch to help support exfat
- Added exfat timestamp conversion + setting
- Switched to datetime variable name for consistency with the header
- Switched to tabs-for-alignment for consistency in the file
grub-core/fs/fat.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
index dc493add2..bacf9e60f 100644
--- a/grub-core/fs/fat.c
+++ b/grub-core/fs/fat.c
@@ -26,6 +26,7 @@
#include <grub/err.h>
#include <grub/dl.h>
#include <grub/charset.h>
+#include <grub/datetime.h>
#ifndef MODE_EXFAT
#include <grub/fat.h>
#else
@@ -730,6 +731,28 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
return grub_errno ? : GRUB_ERR_EOF;
}
+static int
+grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int32_t *nix) {
+ struct grub_datetime datetime = {
+ .year = (field >> 25) + 1980,
+ .month = (field & 0x01E00000) >> 21,
+ .day = (field & 0x001F0000) >> 16,
+ .hour = (field & 0x0000F800) >> 11,
+ .minute = (field & 0x000007E0) >> 5,
+ .second = (field & 0x0000001F) * 2 + (msec >= 100 ? 1 : 0),
+ };
+
+ /* The conversion below allows seconds=60, so don't trust its validation. */
+ if ((field & 0x1F) > 29)
+ return 0;
+
+ /* Validate the 10-msec field even though it is rounded down to seconds. */
+ if (msec > 199)
+ return 0;
+
+ return grub_datetime2unixtime (&datetime, nix);
+}
+
#else
static grub_err_t
@@ -857,6 +880,24 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
return grub_errno ? : GRUB_ERR_EOF;
}
+static int
+grub_fat_timestamp (grub_uint16_t time, grub_uint16_t date, grub_int32_t *nix) {
+ struct grub_datetime datetime = {
+ .year = (date >> 9) + 1980,
+ .month = (date & 0x01E0) >> 5,
+ .day = (date & 0x001F),
+ .hour = (time >> 11),
+ .minute = (time & 0x07E0) >> 5,
+ .second = (time & 0x001F) * 2,
+ };
+
+ /* The conversion below allows seconds=60, so don't trust its validation. */
+ if ((time & 0x1F) > 29)
+ return 0;
+
+ return grub_datetime2unixtime (&datetime, nix);
+}
+
#endif
static grub_err_t lookup_file (grub_fshelp_node_t node,
@@ -966,9 +1007,15 @@ grub_fat_dir (grub_device_t device, const char *path, grub_fs_dir_hook_t hook,
#ifdef MODE_EXFAT
if (!ctxt.dir.have_stream)
continue;
+ info.mtimeset = grub_exfat_timestamp (grub_le_to_cpu32 (ctxt.entry.type_specific.file.m_time),
+ ctxt.entry.type_specific.file.m_time_tenth,
+ &info.mtime);
#else
if (ctxt.dir.attr & GRUB_FAT_ATTR_VOLUME_ID)
continue;
+ info.mtimeset = grub_fat_timestamp (grub_le_to_cpu16 (ctxt.dir.w_time),
+ grub_le_to_cpu16 (ctxt.dir.w_date),
+ &info.mtime);
#endif
if (hook (ctxt.filename, &info, hook_data))
--
2.21.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] fat: Support file modification times
2020-03-03 19:41 [PATCH v2 2/2] fat: Support file modification times David Michael
@ 2020-03-06 12:19 ` Daniel Kiper
2020-03-06 15:48 ` David Michael
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Kiper @ 2020-03-06 12:19 UTC (permalink / raw)
To: David Michael; +Cc: grub-devel
On Tue, Mar 03, 2020 at 02:41:08PM -0500, David Michael wrote:
> This allows comparing file ages on EFI system partitions.
>
> Signed-off-by: David Michael <fedora.dm0@gmail.com>
> ---
>
> Changes since v1:
> - Added the previous patch to help support exfat
> - Added exfat timestamp conversion + setting
> - Switched to datetime variable name for consistency with the header
> - Switched to tabs-for-alignment for consistency in the file
>
> grub-core/fs/fat.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 47 insertions(+)
>
> diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> index dc493add2..bacf9e60f 100644
> --- a/grub-core/fs/fat.c
> +++ b/grub-core/fs/fat.c
> @@ -26,6 +26,7 @@
> #include <grub/err.h>
> #include <grub/dl.h>
> #include <grub/charset.h>
> +#include <grub/datetime.h>
> #ifndef MODE_EXFAT
> #include <grub/fat.h>
> #else
> @@ -730,6 +731,28 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
> return grub_errno ? : GRUB_ERR_EOF;
> }
>
> +static int
> +grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int32_t *nix) {
> + struct grub_datetime datetime = {
> + .year = (field >> 25) + 1980,
> + .month = (field & 0x01E00000) >> 21,
> + .day = (field & 0x001F0000) >> 16,
> + .hour = (field & 0x0000F800) >> 11,
> + .minute = (field & 0x000007E0) >> 5,
> + .second = (field & 0x0000001F) * 2 + (msec >= 100 ? 1 : 0),
> + };
> +
> + /* The conversion below allows seconds=60, so don't trust its validation. */
60 seconds is a valid value in case of leap second. Hence, the question
is: Can 60 seconds be represented properly in exFAT somehow? OK, this
does not happen often. So, if we want ignore that case then at least
I would like to have an explanation that we ignore it due to...
> + if ((field & 0x1F) > 29)
> + return 0;
You silently ignore this error. Should not you spit something to the
console in this case? Or maybe at least set grub_errno? This way
user will know that result of comparison should not be trusted...
> + /* Validate the 10-msec field even though it is rounded down to seconds. */
> + if (msec > 199)
> + return 0;
Ditto...
> + return grub_datetime2unixtime (&datetime, nix);
> +}
> +
> #else
>
> static grub_err_t
> @@ -857,6 +880,24 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
> return grub_errno ? : GRUB_ERR_EOF;
> }
>
> +static int
> +grub_fat_timestamp (grub_uint16_t time, grub_uint16_t date, grub_int32_t *nix) {
> + struct grub_datetime datetime = {
> + .year = (date >> 9) + 1980,
> + .month = (date & 0x01E0) >> 5,
> + .day = (date & 0x001F),
> + .hour = (time >> 11),
> + .minute = (time & 0x07E0) >> 5,
> + .second = (time & 0x001F) * 2,
> + };
> +
> + /* The conversion below allows seconds=60, so don't trust its validation. */
> + if ((time & 0x1F) > 29)
> + return 0;
Ditto...
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] fat: Support file modification times
2020-03-06 12:19 ` Daniel Kiper
@ 2020-03-06 15:48 ` David Michael
2020-03-06 16:09 ` Daniel Kiper
0 siblings, 1 reply; 4+ messages in thread
From: David Michael @ 2020-03-06 15:48 UTC (permalink / raw)
To: Daniel Kiper; +Cc: The development of GNU GRUB
On Fri, Mar 6, 2020 at 7:19 AM Daniel Kiper <dkiper@net-space.pl> wrote:
> On Tue, Mar 03, 2020 at 02:41:08PM -0500, David Michael wrote:
> > This allows comparing file ages on EFI system partitions.
> >
> > Signed-off-by: David Michael <fedora.dm0@gmail.com>
> > ---
> >
> > Changes since v1:
> > - Added the previous patch to help support exfat
> > - Added exfat timestamp conversion + setting
> > - Switched to datetime variable name for consistency with the header
> > - Switched to tabs-for-alignment for consistency in the file
> >
> > grub-core/fs/fat.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 47 insertions(+)
> >
> > diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> > index dc493add2..bacf9e60f 100644
> > --- a/grub-core/fs/fat.c
> > +++ b/grub-core/fs/fat.c
> > @@ -26,6 +26,7 @@
> > #include <grub/err.h>
> > #include <grub/dl.h>
> > #include <grub/charset.h>
> > +#include <grub/datetime.h>
> > #ifndef MODE_EXFAT
> > #include <grub/fat.h>
> > #else
> > @@ -730,6 +731,28 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
> > return grub_errno ? : GRUB_ERR_EOF;
> > }
> >
> > +static int
> > +grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int32_t *nix) {
> > + struct grub_datetime datetime = {
> > + .year = (field >> 25) + 1980,
> > + .month = (field & 0x01E00000) >> 21,
> > + .day = (field & 0x001F0000) >> 16,
> > + .hour = (field & 0x0000F800) >> 11,
> > + .minute = (field & 0x000007E0) >> 5,
> > + .second = (field & 0x0000001F) * 2 + (msec >= 100 ? 1 : 0),
> > + };
> > +
> > + /* The conversion below allows seconds=60, so don't trust its validation. */
>
> 60 seconds is a valid value in case of leap second. Hence, the question
> is: Can 60 seconds be represented properly in exFAT somehow? OK, this
> does not happen often. So, if we want ignore that case then at least
> I would like to have an explanation that we ignore it due to...
I enforced the 0-59 range because that is what is declared valid in
the spec. See 11.3.5 in ECMA-107[1] and 7.4.8 for exfat[2].
> > + if ((field & 0x1F) > 29)
> > + return 0;
>
> You silently ignore this error. Should not you spit something to the
> console in this case? Or maybe at least set grub_errno? This way
> user will know that result of comparison should not be trusted...
The functions also rely on the grub_datetime2unixtime field
validations, which do not print errors. I can add a general
grub_error if info.mtimeset is zero so it warns of all failures.
Thanks.
David
[1] https://www.ecma-international.org/publications/files/ECMA-ST/Ecma-107.pdf
[2] https://docs.microsoft.com/en-us/windows/win32/fileio/exfat-specification
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] fat: Support file modification times
2020-03-06 15:48 ` David Michael
@ 2020-03-06 16:09 ` Daniel Kiper
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Kiper @ 2020-03-06 16:09 UTC (permalink / raw)
To: David Michael; +Cc: The development of GNU GRUB
On Fri, Mar 06, 2020 at 10:48:07AM -0500, David Michael wrote:
> On Fri, Mar 6, 2020 at 7:19 AM Daniel Kiper <dkiper@net-space.pl> wrote:
> > On Tue, Mar 03, 2020 at 02:41:08PM -0500, David Michael wrote:
> > > This allows comparing file ages on EFI system partitions.
> > >
> > > Signed-off-by: David Michael <fedora.dm0@gmail.com>
> > > ---
> > >
> > > Changes since v1:
> > > - Added the previous patch to help support exfat
> > > - Added exfat timestamp conversion + setting
> > > - Switched to datetime variable name for consistency with the header
> > > - Switched to tabs-for-alignment for consistency in the file
> > >
> > > grub-core/fs/fat.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> > > 1 file changed, 47 insertions(+)
> > >
> > > diff --git a/grub-core/fs/fat.c b/grub-core/fs/fat.c
> > > index dc493add2..bacf9e60f 100644
> > > --- a/grub-core/fs/fat.c
> > > +++ b/grub-core/fs/fat.c
> > > @@ -26,6 +26,7 @@
> > > #include <grub/err.h>
> > > #include <grub/dl.h>
> > > #include <grub/charset.h>
> > > +#include <grub/datetime.h>
> > > #ifndef MODE_EXFAT
> > > #include <grub/fat.h>
> > > #else
> > > @@ -730,6 +731,28 @@ grub_fat_iterate_dir_next (grub_fshelp_node_t node,
> > > return grub_errno ? : GRUB_ERR_EOF;
> > > }
> > >
> > > +static int
> > > +grub_exfat_timestamp (grub_uint32_t field, grub_uint8_t msec, grub_int32_t *nix) {
> > > + struct grub_datetime datetime = {
> > > + .year = (field >> 25) + 1980,
> > > + .month = (field & 0x01E00000) >> 21,
> > > + .day = (field & 0x001F0000) >> 16,
> > > + .hour = (field & 0x0000F800) >> 11,
> > > + .minute = (field & 0x000007E0) >> 5,
> > > + .second = (field & 0x0000001F) * 2 + (msec >= 100 ? 1 : 0),
> > > + };
> > > +
> > > + /* The conversion below allows seconds=60, so don't trust its validation. */
> >
> > 60 seconds is a valid value in case of leap second. Hence, the question
> > is: Can 60 seconds be represented properly in exFAT somehow? OK, this
> > does not happen often. So, if we want ignore that case then at least
> > I would like to have an explanation that we ignore it due to...
>
> I enforced the 0-59 range because that is what is declared valid in
> the spec. See 11.3.5 in ECMA-107[1] and 7.4.8 for exfat[2].
OK, could you add references to these documents into the comments?
> > > + if ((field & 0x1F) > 29)
> > > + return 0;
> >
> > You silently ignore this error. Should not you spit something to the
> > console in this case? Or maybe at least set grub_errno? This way
> > user will know that result of comparison should not be trusted...
>
> The functions also rely on the grub_datetime2unixtime field
> validations, which do not print errors. I can add a general
> grub_error if info.mtimeset is zero so it warns of all failures.
Works for me.
Daniel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-06 16:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-03 19:41 [PATCH v2 2/2] fat: Support file modification times David Michael
2020-03-06 12:19 ` Daniel Kiper
2020-03-06 15:48 ` David Michael
2020-03-06 16:09 ` Daniel Kiper
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.