linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior
@ 2018-07-10 21:40 Arnd Bergmann
  2018-07-10 21:40 ` [PATCH 2/2] [v2] hfs/hfsplus: stop using timespec based interfaces Arnd Bergmann
  2018-07-11 22:46 ` [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Ernesto A. Fernández
  0 siblings, 2 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-07-10 21:40 UTC (permalink / raw)
  To: Andrew Morton
  Cc: y2038, Al Viro, Arnd Bergmann, stable, Thomas Gleixner,
	Vyacheslav Dubeyko, linux-fsdevel, linux-kernel

According to the official documentation for HFS+ [1], inode timestamps
are supposed to cover the time range from 1904 to 2040 as originally
used in classic MacOS.

The traditional Linux usage is to convert the timestamps into an unsigned
32-bit number based on the Unix epoch and from there to a time_t. On
32-bit systems, that wraps the time from 2038 to 1902, so the last
two years of the valid time range become garbled. On 64-bit systems,
all times before 1970 get turned into timestamps between 2038 and 2106,
which is more convenient but also different from the documented behavior.

Looking at the Darwin sources [2], it seems that MacOS is inconsistent in
yet another way: all timestamps are wrapped around to a 32-bit unsigned
number when written to the disk, but when read back, all numeric values
lower than 2082844800U are assumed to be invalid, so we cannot represent
the times before 1970 or the times after 2040.

While all implementations seem to agree on the interpretation of values
between 1970 and 2038, they often differ on the exact range they support
when reading back values outside of the common range:

MacOS (traditional):		1904-2040
Apple Documentation:		1904-2040
MacOS X source comments:	1970-2040
MacOS X source code:		1970-2038
32-bit Linux:			1902-2038
64-bit Linux:			1970-2106
hfsfuse:			1970-2040
hfsutils (32 bit, old libc)	1902-2038
hfsutils (32 bit, new libc)	1970-2106
hfsutils (64 bit)		1904-2040
hfsplus-utils			1904-2040
hfsexplorer			1904-2040
7-zip				1904-2040

This changes Linux over to mostly the same behavior as described in the
code comment in MacOS X, disallowing all times before 1970 and after
2040, while still allowing times between 2038 and 2040 like most other
implementations do. Most importantly, it means we can have the same
behavior on 32-bit and 64-bit.

Cc: stable@vger.kernel.org
Link: [1] https://developer.apple.com/library/archive/technotes/tn/tn1150.html
Link: [2] https://opensource.apple.com/source/hfs/hfs-407.30.1/core/MacOSStubs.c.auto.html
Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: treat pre-1970 dates as invalid following MacOS X behavior,
    reword and expand changelog text
---
 fs/hfs/hfs_fs.h         | 29 +++++++++++++++++++++++++----
 fs/hfsplus/hfsplus_fs.h | 26 +++++++++++++++++++++++---
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index 6d0783e2e276..1af998fb522e 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -246,14 +246,35 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
  *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
  *
  */
-#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)
+static inline time64_t __hfs_m_to_utime(__be32 mt)
+{
+	time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
+
+	/*
+	 * Times past 2040-02-06 06:28 are assumed to be invalid,
+	 * matching the MacOS behavior.
+	 */
+	if (ut > 2082844800U + UINT_MAX)
+		ut = 0;
+
+	return ut + sys_tz.tz_minuteswest * 60;
+}
 
+static inline __be32 __hfs_u_to_mtime(time64_t ut)
+{
+	ut -= - sys_tz.tz_minuteswest * 60;
+
+	/*
+	 * MacOS wraps "invalid" times after 2040 when writing back, so
+	 * let's do the same here.
+	 */
+	return cpu_to_be32(lower_32_bits(ut + 2082844800U));
+}
 #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)
 
-#define hfs_m_to_utime(time)	(struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
-#define hfs_u_to_mtime(time)	__hfs_u_to_mtime((time).tv_sec)
+#define hfs_m_to_utime(time)   (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
+#define hfs_u_to_mtime(time)   __hfs_u_to_mtime((time).tv_sec)
 #define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
 
 static inline const char *hfs_mdb_name(struct super_block *sb)
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index d9255abafb81..7f0943e540a0 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -530,9 +530,29 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
 		       void **data, int op, int op_flags);
 int hfsplus_read_wrapper(struct super_block *sb);
 
-/* time macros */
-#define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
-#define __hfsp_ut2mt(t)		(cpu_to_be32(t + 2082844800U))
+/* time helpers */
+static inline time64_t __hfsp_mt2ut(__be32 mt)
+{
+	time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
+
+	/*
+	 * Times past 2040-02-06 06:28 are assumed to be invalid,
+	 * matching the MacOS behavior.
+	 */
+	if (ut > 2082844800U + UINT_MAX)
+		ut = 0;
+
+	return ut;
+}
+
+static inline __be32 __hfsp_ut2mt(time64_t ut)
+{
+	/*
+	 * MacOS wraps "invalid" times after 2040 when writing back, so
+	 * let's do the same here.
+	 */
+	return cpu_to_be32(lower_32_bits(ut + 2082844800U));
+}
 
 /* compatibility */
 #define hfsp_mt2ut(t)		(struct timespec){ .tv_sec = __hfsp_mt2ut(t) }
-- 
2.9.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] [v2] hfs/hfsplus: stop using timespec based interfaces
  2018-07-10 21:40 [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Arnd Bergmann
@ 2018-07-10 21:40 ` Arnd Bergmann
  2018-07-11 22:46 ` [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Ernesto A. Fernández
  1 sibling, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-07-10 21:40 UTC (permalink / raw)
  To: Andrew Morton
  Cc: y2038, Al Viro, Arnd Bergmann, Deepa Dinamani, Jeff Layton,
	Jan Kara, Vyacheslav Dubeyko, Ernesto A. Fernandez,
	Thomas Gleixner, linux-fsdevel, linux-kernel

The native HFS and HFS+ timestamps overflow in year 2040, two years after
the Unix y2038 overflow. On 32-bit systems, we currently go through a
conversion that overflows in 2038, while the VFS code is now capable of
representing a 64-bit time range.

This removes the unnecessary conversion, using 64-bit timestamps
consistently

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: reword changelog, combine hfs and hfs+ patches
---
 fs/hfs/hfs_fs.h         |  4 ++--
 fs/hfs/inode.c          |  4 ++--
 fs/hfsplus/hfsplus_fs.h |  4 ++--
 fs/hfsplus/inode.c      | 12 ++++++------
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index 1af998fb522e..45cf1bccc1f2 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -273,9 +273,9 @@ static inline __be32 __hfs_u_to_mtime(time64_t ut)
 #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)
 
-#define hfs_m_to_utime(time)   (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
+#define hfs_m_to_utime(time)   (struct timespec64){ .tv_sec = __hfs_m_to_utime(time) }
 #define hfs_u_to_mtime(time)   __hfs_u_to_mtime((time).tv_sec)
-#define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
+#define hfs_mtime()		__hfs_u_to_mtime(ktime_get_real_seconds())
 
 static inline const char *hfs_mdb_name(struct super_block *sb)
 {
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 2a16111d312f..b3309b83371a 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -351,7 +351,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		inode->i_mode &= ~hsb->s_file_umask;
 		inode->i_mode |= S_IFREG;
 		inode->i_ctime = inode->i_atime = inode->i_mtime =
-				timespec_to_timespec64(hfs_m_to_utime(rec->file.MdDat));
+				hfs_m_to_utime(rec->file.MdDat);
 		inode->i_op = &hfs_file_inode_operations;
 		inode->i_fop = &hfs_file_operations;
 		inode->i_mapping->a_ops = &hfs_aops;
@@ -362,7 +362,7 @@ static int hfs_read_inode(struct inode *inode, void *data)
 		HFS_I(inode)->fs_blocks = 0;
 		inode->i_mode = S_IFDIR | (S_IRWXUGO & ~hsb->s_dir_umask);
 		inode->i_ctime = inode->i_atime = inode->i_mtime =
-				timespec_to_timespec64(hfs_m_to_utime(rec->dir.MdDat));
+				hfs_m_to_utime(rec->dir.MdDat);
 		inode->i_op = &hfs_dir_inode_operations;
 		inode->i_fop = &hfs_dir_operations;
 		break;
diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
index 7f0943e540a0..677ef7f9204d 100644
--- a/fs/hfsplus/hfsplus_fs.h
+++ b/fs/hfsplus/hfsplus_fs.h
@@ -555,8 +555,8 @@ static inline __be32 __hfsp_ut2mt(time64_t ut)
 }
 
 /* compatibility */
-#define hfsp_mt2ut(t)		(struct timespec){ .tv_sec = __hfsp_mt2ut(t) }
+#define hfsp_mt2ut(t)		(struct timespec64){ .tv_sec = __hfsp_mt2ut(t) }
 #define hfsp_ut2mt(t)		__hfsp_ut2mt((t).tv_sec)
-#define hfsp_now2mt()		__hfsp_ut2mt(get_seconds())
+#define hfsp_now2mt()		__hfsp_ut2mt(ktime_get_real_seconds())
 
 #endif
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index c824f702feec..c0c8d433864f 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -493,9 +493,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 		hfsplus_get_perms(inode, &folder->permissions, 1);
 		set_nlink(inode, 1);
 		inode->i_size = 2 + be32_to_cpu(folder->valence);
-		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(folder->access_date));
-		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(folder->content_mod_date));
-		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(folder->attribute_mod_date));
+		inode->i_atime = hfsp_mt2ut(folder->access_date);
+		inode->i_mtime = hfsp_mt2ut(folder->content_mod_date);
+		inode->i_ctime = hfsp_mt2ut(folder->attribute_mod_date);
 		HFSPLUS_I(inode)->create_date = folder->create_date;
 		HFSPLUS_I(inode)->fs_blocks = 0;
 		if (folder->flags & cpu_to_be16(HFSPLUS_HAS_FOLDER_COUNT)) {
@@ -531,9 +531,9 @@ int hfsplus_cat_read_inode(struct inode *inode, struct hfs_find_data *fd)
 			init_special_inode(inode, inode->i_mode,
 					   be32_to_cpu(file->permissions.dev));
 		}
-		inode->i_atime = timespec_to_timespec64(hfsp_mt2ut(file->access_date));
-		inode->i_mtime = timespec_to_timespec64(hfsp_mt2ut(file->content_mod_date));
-		inode->i_ctime = timespec_to_timespec64(hfsp_mt2ut(file->attribute_mod_date));
+		inode->i_atime = hfsp_mt2ut(file->access_date);
+		inode->i_mtime = hfsp_mt2ut(file->content_mod_date);
+		inode->i_ctime = hfsp_mt2ut(file->attribute_mod_date);
 		HFSPLUS_I(inode)->create_date = file->create_date;
 	} else {
 		pr_err("bad catalog entry used to create inode\n");
-- 
2.9.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior
  2018-07-10 21:40 [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Arnd Bergmann
  2018-07-10 21:40 ` [PATCH 2/2] [v2] hfs/hfsplus: stop using timespec based interfaces Arnd Bergmann
@ 2018-07-11 22:46 ` Ernesto A. Fernández
  2018-07-24 15:28   ` Arnd Bergmann
  1 sibling, 1 reply; 4+ messages in thread
From: Ernesto A. Fernández @ 2018-07-11 22:46 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, y2038, Al Viro, stable, Thomas Gleixner,
	Vyacheslav Dubeyko, linux-fsdevel, linux-kernel

Hi:

On Tue, Jul 10, 2018 at 11:40:50PM +0200, Arnd Bergmann wrote:
> According to the official documentation for HFS+ [1], inode timestamps
> are supposed to cover the time range from 1904 to 2040 as originally
> used in classic MacOS.
> 
> The traditional Linux usage is to convert the timestamps into an unsigned
> 32-bit number based on the Unix epoch and from there to a time_t. On
> 32-bit systems, that wraps the time from 2038 to 1902, so the last
> two years of the valid time range become garbled. On 64-bit systems,
> all times before 1970 get turned into timestamps between 2038 and 2106,
> which is more convenient but also different from the documented behavior.
> 
> Looking at the Darwin sources [2], it seems that MacOS is inconsistent in
> yet another way: all timestamps are wrapped around to a 32-bit unsigned
> number when written to the disk, but when read back, all numeric values
> lower than 2082844800U are assumed to be invalid, so we cannot represent
> the times before 1970 or the times after 2040.
> 
> While all implementations seem to agree on the interpretation of values
> between 1970 and 2038, they often differ on the exact range they support
> when reading back values outside of the common range:
> 
> MacOS (traditional):		1904-2040
> Apple Documentation:		1904-2040
> MacOS X source comments:	1970-2040
> MacOS X source code:		1970-2038
> 32-bit Linux:			1902-2038
> 64-bit Linux:			1970-2106
> hfsfuse:			1970-2040
> hfsutils (32 bit, old libc)	1902-2038
> hfsutils (32 bit, new libc)	1970-2106
> hfsutils (64 bit)		1904-2040
> hfsplus-utils			1904-2040
> hfsexplorer			1904-2040
> 7-zip				1904-2040
> 
> This changes Linux over to mostly the same behavior as described in the
> code comment in MacOS X, disallowing all times before 1970 and after
> 2040, while still allowing times between 2038 and 2040 like most other
> implementations do. Most importantly, it means we can have the same
> behavior on 32-bit and 64-bit.
> 
> Cc: stable@vger.kernel.org
> Link: [1] https://developer.apple.com/library/archive/technotes/tn/tn1150.html
> Link: [2] https://opensource.apple.com/source/hfs/hfs-407.30.1/core/MacOSStubs.c.auto.html
> Suggested-by: Viacheslav Dubeyko <slava@dubeyko.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: treat pre-1970 dates as invalid following MacOS X behavior,
>     reword and expand changelog text
> ---
>  fs/hfs/hfs_fs.h         | 29 +++++++++++++++++++++++++----
>  fs/hfsplus/hfsplus_fs.h | 26 +++++++++++++++++++++++---
>  2 files changed, 48 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
> index 6d0783e2e276..1af998fb522e 100644
> --- a/fs/hfs/hfs_fs.h
> +++ b/fs/hfs/hfs_fs.h
> @@ -246,14 +246,35 @@ extern void hfs_mark_mdb_dirty(struct super_block *sb);
>   *	mac:	unsigned big-endian since 00:00 GMT, Jan. 1, 1904
>   *
>   */
> -#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)
> +static inline time64_t __hfs_m_to_utime(__be32 mt)
> +{
> +	time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
> +
> +	/*
> +	 * Times past 2040-02-06 06:28 are assumed to be invalid,
> +	 * matching the MacOS behavior.
> +	 */
> +	if (ut > 2082844800U + UINT_MAX)

I'm not sure what you were going for here, but this isn't right. Times
as early as 2036 will be considered invalid.

> +		ut = 0;
> +
> +	return ut + sys_tz.tz_minuteswest * 60;
> +}
>  
> +static inline __be32 __hfs_u_to_mtime(time64_t ut)
> +{
> +	ut -= - sys_tz.tz_minuteswest * 60;
	   ^^^^^
	An extra minus.

> +
> +	/*
> +	 * MacOS wraps "invalid" times after 2040 when writing back, so
> +	 * let's do the same here.
> +	 */
> +	return cpu_to_be32(lower_32_bits(ut + 2082844800U));
> +}
>  #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)
>  
> -#define hfs_m_to_utime(time)	(struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
> -#define hfs_u_to_mtime(time)	__hfs_u_to_mtime((time).tv_sec)
> +#define hfs_m_to_utime(time)   (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
> +#define hfs_u_to_mtime(time)   __hfs_u_to_mtime((time).tv_sec)

Are the new spaces intentional?

>  #define hfs_mtime()		__hfs_u_to_mtime(get_seconds())
>  
>  static inline const char *hfs_mdb_name(struct super_block *sb)
> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
> index d9255abafb81..7f0943e540a0 100644
> --- a/fs/hfsplus/hfsplus_fs.h
> +++ b/fs/hfsplus/hfsplus_fs.h
> @@ -530,9 +530,29 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
>  		       void **data, int op, int op_flags);
>  int hfsplus_read_wrapper(struct super_block *sb);
>  
> -/* time macros */
> -#define __hfsp_mt2ut(t)		(be32_to_cpu(t) - 2082844800U)
> -#define __hfsp_ut2mt(t)		(cpu_to_be32(t + 2082844800U))
> +/* time helpers */
> +static inline time64_t __hfsp_mt2ut(__be32 mt)
> +{
> +	time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
> +
> +	/*
> +	 * Times past 2040-02-06 06:28 are assumed to be invalid,
> +	 * matching the MacOS behavior.
> +	 */
> +	if (ut > 2082844800U + UINT_MAX)

Same as before, 2036-2040 will be invalid.


For the record, your original solution (supporting the 1970-2106 range)
still makes more sense to me. It seems Apple is not using the 1904-1970
range for anything; if they are still supporting hfsplus by the 2030s I
assume they will deal with this in a similar way.

Thanks,
Ernest

> +		ut = 0;
> +
> +	return ut;
> +}
> +
> +static inline __be32 __hfsp_ut2mt(time64_t ut)
> +{
> +	/*
> +	 * MacOS wraps "invalid" times after 2040 when writing back, so
> +	 * let's do the same here.
> +	 */
> +	return cpu_to_be32(lower_32_bits(ut + 2082844800U));
> +}
>  
>  /* compatibility */
>  #define hfsp_mt2ut(t)		(struct timespec){ .tv_sec = __hfsp_mt2ut(t) }
> -- 
> 2.9.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior
  2018-07-11 22:46 ` [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Ernesto A. Fernández
@ 2018-07-24 15:28   ` Arnd Bergmann
  0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-07-24 15:28 UTC (permalink / raw)
  To: Ernesto A. Fernández
  Cc: Andrew Morton, y2038 Mailman List, Al Viro, # 3.4.x,
	Thomas Gleixner, Vyacheslav Dubeyko, Linux FS-devel Mailing List,
	Linux Kernel Mailing List

On Thu, Jul 12, 2018 at 12:46 AM, Ernesto A. Fernández
<ernesto.mnd.fernandez@gmail.com> wrote:

>> -#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)
>> +static inline time64_t __hfs_m_to_utime(__be32 mt)
>> +{
>> +     time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
>> +
>> +     /*
>> +      * Times past 2040-02-06 06:28 are assumed to be invalid,
>> +      * matching the MacOS behavior.
>> +      */
>> +     if (ut > 2082844800U + UINT_MAX)
>
> I'm not sure what you were going for here, but this isn't right. Times
> as early as 2036 will be considered invalid.

Right, the calculation makes no sense, I have no idea how I ever
got there. I was trying to check for the 2040 date, and rearranged my
code multiple times for that. What I probably meant was

     if (ut + 2082844800U > UINT_MAX)

or

    if (ut > UINT_MAX - 2082844800U)

>> +             ut = 0;
>> +
>> +     return ut + sys_tz.tz_minuteswest * 60;
>> +}
>>
>> +static inline __be32 __hfs_u_to_mtime(time64_t ut)
>> +{
>> +     ut -= - sys_tz.tz_minuteswest * 60;
>            ^^^^^
>         An extra minus.

I saw that Andrew had added a fix on top and assumed he got it right, but
upon checking again later I see that it's still there.

>> +
>> +     /*
>> +      * MacOS wraps "invalid" times after 2040 when writing back, so
>> +      * let's do the same here.
>> +      */
>> +     return cpu_to_be32(lower_32_bits(ut + 2082844800U));
>> +}
>>  #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)
>>
>> -#define hfs_m_to_utime(time) (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
>> -#define hfs_u_to_mtime(time) __hfs_u_to_mtime((time).tv_sec)
>> +#define hfs_m_to_utime(time)   (struct timespec){ .tv_sec = __hfs_m_to_utime(time) }
>> +#define hfs_u_to_mtime(time)   __hfs_u_to_mtime((time).tv_sec)
>
> Are the new spaces intentional?

No, I should drop that.

>>  #define hfs_mtime()          __hfs_u_to_mtime(get_seconds())
>>
>>  static inline const char *hfs_mdb_name(struct super_block *sb)
>> diff --git a/fs/hfsplus/hfsplus_fs.h b/fs/hfsplus/hfsplus_fs.h
>> index d9255abafb81..7f0943e540a0 100644
>> --- a/fs/hfsplus/hfsplus_fs.h
>> +++ b/fs/hfsplus/hfsplus_fs.h
>> @@ -530,9 +530,29 @@ int hfsplus_submit_bio(struct super_block *sb, sector_t sector, void *buf,
>>                      void **data, int op, int op_flags);
>>  int hfsplus_read_wrapper(struct super_block *sb);
>>
>> -/* time macros */
>> -#define __hfsp_mt2ut(t)              (be32_to_cpu(t) - 2082844800U)
>> -#define __hfsp_ut2mt(t)              (cpu_to_be32(t + 2082844800U))
>> +/* time helpers */
>> +static inline time64_t __hfsp_mt2ut(__be32 mt)
>> +{
>> +     time64_t ut = (u32)(be32_to_cpu(mt) - 2082844800U);
>> +
>> +     /*
>> +      * Times past 2040-02-06 06:28 are assumed to be invalid,
>> +      * matching the MacOS behavior.
>> +      */
>> +     if (ut > 2082844800U + UINT_MAX)
>
> Same as before, 2036-2040 will be invalid.
>
>
> For the record, your original solution (supporting the 1970-2106 range)
> still makes more sense to me. It seems Apple is not using the 1904-1970
> range for anything; if they are still supporting hfsplus by the 2030s I
> assume they will deal with this in a similar way.

I'm definitely fine with it either way. The current approach was based on
the feedback I got from Viacheslav Dubeyko, and on what I found
in the various other implementations. Viacheslav, are you ok with
changing the hfs code back to my original interpretation of the
timestamps, using the 1970..2106 range?

Andrew, can you drop both my hfs/hfsplus patches for now? At
this point it seems better to start from scratch than to fix up the
bugs individually.

      Arnd

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-07-24 15:28 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-10 21:40 [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Arnd Bergmann
2018-07-10 21:40 ` [PATCH 2/2] [v2] hfs/hfsplus: stop using timespec based interfaces Arnd Bergmann
2018-07-11 22:46 ` [PATCH 1/2] [v2] hfs/hfsplus: follow MacOS time behavior Ernesto A. Fernández
2018-07-24 15:28   ` Arnd Bergmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).