Messages in this thread | | | Subject | Re: [Y2038] [PATCH 13/16] hfs/hfsplus: use 64-bit inode timestamps | From | Viacheslav Dubeyko <> | Date | Wed, 13 Nov 2019 20:03:56 +0300 |
| |
> On Nov 13, 2019, at 11:06 AM, Arnd Bergmann <arnd@arndb.de> wrote: > > On Wed, Nov 13, 2019 at 7:00 AM Viacheslav Dubeyko <slava@dubeyko.com> wrote: >>> On Nov 9, 2019, at 12:32 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> * There are two time systems. Both are based on seconds since >>> * a particular time/date. >>> - * Unix: unsigned lil-endian since 00:00 GMT, Jan. 1, 1970 >>> + * Unix: signed little-endian since 00:00 GMT, Jan. 1, 1970 >>> * mac: unsigned big-endian since 00:00 GMT, Jan. 1, 1904 >>> * >>> + * HFS implementations are highly inconsistent, this one matches the >>> + * traditional behavior of 64-bit Linux, giving the most useful >>> + * time range between 1970 and 2106, by treating any on-disk timestamp >>> + * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106. >>> */ >>> -#define __hfs_u_to_mtime(sec) cpu_to_be32(sec + 2082844800U - sys_tz.tz_minuteswest * 60) >>> -#define __hfs_m_to_utime(sec) (be32_to_cpu(sec) - 2082844800U + sys_tz.tz_minuteswest * 60) >> >> I believe it makes sense to introduce some constant instead of hardcoded value (2082844800U and 60). >> It will be easier to understand the code without necessity to take a look into the comments. >> What do you think? > > Every other user of sys_tz.tz_minuteswest uses a plain '60', I think that one > is easy enough to understand from context. Naming the other constant > is a good idea, I've now folded the change below into my patch. > > Thanks for the review! > > Arnd > > 8<----- > diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h > index 26733051ee50..f71c384064c8 100644 > --- a/fs/hfs/hfs_fs.h > +++ b/fs/hfs/hfs_fs.h > @@ -247,22 +247,24 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb); > * > * HFS implementations are highly inconsistent, this one matches the > * traditional behavior of 64-bit Linux, giving the most useful > * time range between 1970 and 2106, by treating any on-disk timestamp > - * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106. > + * under HFS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106. > */ > +#define HFS_UTC_OFFSET 2082844800U > + > static inline time64_t __hfs_m_to_utime(__be32 mt) > { > - time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U); > + time64_t ut = (u32)(be32_to_cpu(mt) - HFS_UTC_OFFSET); > > return ut + sys_tz.tz_minuteswest * 60; > } > > static inline __be32 __hfs_u_to_mtime(time64_t ut) > { > ut -= sys_tz.tz_minuteswest * 60; > > - return cpu_to_be32(lower_32_bits(ut) + 2082844800U); > + return cpu_to_be32(lower_32_bits(ut) + HFS_UTC_OFFSET); > } > #define HFS_I(inode) (container_of(inode, struct hfs_inode_info, vfs_inode)) > #define HFS_SB(sb) ((struct hfs_sb_info *)(sb)->s_fs_info) > > diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h > index 22d0a22c41a3..3b03fff68543 100644 > --- a/fs/hfsplus/hfsplus_fs.h > +++ b/fs/hfsplus/hfsplus_fs.h > @@ -538,20 +538,22 @@ int hfsplus_read_wrapper(struct super_block *sb); > * > * HFS+ implementations are highly inconsistent, this one matches the > * traditional behavior of 64-bit Linux, giving the most useful > * time range between 1970 and 2106, by treating any on-disk timestamp > - * under 2082844800U (Jan 1 1970) as a time between 2040 and 2106. > + * under HFSPLUS_UTC_OFFSET (Jan 1 1970) as a time between 2040 and 2106. > */ > +#define HFSPLUS_UTC_OFFSET 2082844800U > + > static inline time64_t __hfsp_mt2ut(__be32 mt) > { > - time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U); > + time64_t ut = (u32)(be32_to_cpu(mt) - HFSPLUS_UTC_OFFSET); > > return ut; > } > > static inline __be32 __hfsp_ut2mt(time64_t ut) > { > - return cpu_to_be32(lower_32_bits(ut) + 2082844800U); > + return cpu_to_be32(lower_32_bits(ut) + HFSPLUS_UTC_OFFSET); > } > > /* compatibility */ > #define hfsp_mt2ut(t) (struct timespec64){ .tv_sec = __hfsp_mt2ut(t) }
Looks good for me. I like the patch.
Reviewed-by: Vyacheslav Dubeyko <slava@dubeyko.com>
Thanks, Vyacheslav Dubeyko.
| |