On Fri, Mar 06, 2020 at 10:48:07AM -0500, David Michael wrote:
> On Fri, Mar 6, 2020 at 7:19 AM Daniel Kiper <dki...@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....@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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
