* [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time
@ 2020-02-07 13:00 Lukasz Majewski
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-07 13:00 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab, Lukasz Majewski
This patch series provides __utime64, __utimes64 and struct __utimbuf64.
The branch with integrated Y2038 support:
https://github.com/lmajewski/y2038_glibc/commits/glibc_utime_conversion_v1
Lukasz Majewski (3):
y2038: Introduce struct __utimbuf64 - new internal glibc type
y2038: linux: Provide __utimes64 implementation
y2038: linux: Provide __utime64 implementation
include/time.h | 18 +++++++++
sysdeps/unix/sysv/linux/syscalls.list | 1 -
sysdeps/unix/sysv/linux/utime.c | 56 +++++++++++++++++++++++++++
sysdeps/unix/sysv/linux/utimes.c | 37 ++++++++++++------
4 files changed, 100 insertions(+), 12 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/utime.c
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type
2020-02-07 13:00 [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
@ 2020-02-07 13:00 ` Lukasz Majewski
2020-02-07 20:52 ` Alistair Francis
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-07 13:00 ` [PATCH 2/3] y2038: linux: Provide __utimes64 implementation Lukasz Majewski
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-07 13:00 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab, Lukasz Majewski
This type is a glibc's "internal" type to store file's access and modification
times in __time64_t rather than __time_t, which makes it Y2038-proof.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
include/time.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/time.h b/include/time.h
index 389355a5a5..e38f5e32e6 100644
--- a/include/time.h
+++ b/include/time.h
@@ -109,6 +109,18 @@ struct __timeval64
};
#endif
+#if __TIMESIZE == 64
+# define __utimbuf64 utimbuf
+#else
+/* The glibc Y2038-proof struct __utimbuf64 structure for file's access
+ and modification time values. */
+struct __utimbuf64
+{
+ __time64_t actime; /* Access time. */
+ __time64_t modtime; /* Modification time. */
+};
+#endif
+
#if __TIMESIZE == 64
# define __itimerval64 itimerval
#else
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] y2038: linux: Provide __utimes64 implementation
2020-02-07 13:00 [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
@ 2020-02-07 13:00 ` Lukasz Majewski
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-07 13:00 ` [PATCH 3/3] y2038: linux: Provide __utime64 implementation Lukasz Majewski
2020-02-14 8:56 ` [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
3 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-07 13:00 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab, Lukasz Majewski
This patch provides new __utimes64 explicit 64 bit function for setting file's
64 bit attributes for access and modification time.
Internally, the __utimensat64_helper function is used. This patch is necessary
for having architectures with __WORDSIZE == 32 Y2038 safe.
Moreover, a 32 bit version - __utimes has been refactored to internally use
__utimes64.
The __utimes is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
timeval to 64 bit struct __timeval64.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
https://github.com/lmajewski/meta-y2038 and run tests:
https://github.com/lmajewski/y2038-tests/commits/master
Above tests were performed with Y2038 redirection applied as well as without
to test proper usage of both __utimes64 and __utimes.
---
include/time.h | 3 +++
sysdeps/unix/sysv/linux/utimes.c | 37 ++++++++++++++++++++++----------
2 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/include/time.h b/include/time.h
index e38f5e32e6..b04747889a 100644
--- a/include/time.h
+++ b/include/time.h
@@ -211,8 +211,11 @@ libc_hidden_proto (__clock_getres64);
#endif
#if __TIMESIZE == 64
+# define __utimes64 __utimes
# define __utimensat64 __utimensat
#else
+extern int __utimes64 (const char *file, const struct __timeval64 tvp[2]);
+libc_hidden_proto (__utimes64)
extern int __utimensat64 (int fd, const char *file,
const struct __timespec64 tsp[2], int flags);
libc_hidden_proto (__utimensat64);
diff --git a/sysdeps/unix/sysv/linux/utimes.c b/sysdeps/unix/sysv/linux/utimes.c
index 121d883469..09c4e56f18 100644
--- a/sysdeps/unix/sysv/linux/utimes.c
+++ b/sysdeps/unix/sysv/linux/utimes.c
@@ -16,22 +16,37 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
-#include <errno.h>
-#include <stddef.h>
-#include <utime.h>
-#include <sys/time.h>
-#include <sysdep.h>
+#include <time.h>
+int
+__utimes64 (const char *file, const struct __timeval64 tvp[2])
+{
+ struct __timespec64 ts64[2];
+
+ if (tvp)
+ {
+ ts64[0] = timeval64_to_timespec64 (tvp[0]);
+ ts64[1] = timeval64_to_timespec64 (tvp[1]);
+ }
+
+ return __utimensat64_helper (0, file, tvp ? ts64 : NULL, 0);
+}
-/* Consider moving to syscalls.list. */
+#if __TIMESIZE != 64
+libc_hidden_def (__utimes64)
-/* Change the access time of FILE to TVP[0] and
- the modification time of FILE to TVP[1]. */
int
__utimes (const char *file, const struct timeval tvp[2])
{
- /* Avoid implicit array coercion in syscall macros. */
- return INLINE_SYSCALL (utimes, 2, file, &tvp[0]);
-}
+ struct __timeval64 tv64[2];
+ if (tvp)
+ {
+ tv64[0] = valid_timeval_to_timeval64 (tvp[0]);
+ tv64[1] = valid_timeval_to_timeval64 (tvp[1]);
+ }
+
+ return __utimes64 (file, tvp ? tv64 : NULL);
+}
+#endif
weak_alias (__utimes, utimes)
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] y2038: linux: Provide __utime64 implementation
2020-02-07 13:00 [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
2020-02-07 13:00 ` [PATCH 2/3] y2038: linux: Provide __utimes64 implementation Lukasz Majewski
@ 2020-02-07 13:00 ` Lukasz Majewski
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-14 8:56 ` [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
3 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-07 13:00 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab, Lukasz Majewski
This patch replaces auto generated wrapper (as described in
sysdeps/unix/sysv/linux/syscalls.list) for utime with one which adds extra
support for setting file's access and modification 64 bit time on machines
with __TIMESIZE != 64.
Internally, the __utimensat_time64 helper function is used. This patch is
necessary for having architectures with __WORDSIZE == 32 && __TIMESIZE != 64
Y2038 safe.
Moreover, a 32 bit version - __utime has been refactored to internally use
__utime64.
The __utime is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary conversion between struct
utimbuf and struct __utimbuf64.
Build tests:
./src/scripts/build-many-glibcs.py glibcs
Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
https://github.com/lmajewski/meta-y2038 and run tests:
https://github.com/lmajewski/y2038-tests/commits/master
Above tests were performed with Y2038 redirection applied as well as
without to test proper usage of both __utime64 and __utime.
---
include/time.h | 3 ++
sysdeps/unix/sysv/linux/syscalls.list | 1 -
sysdeps/unix/sysv/linux/utime.c | 56 +++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 sysdeps/unix/sysv/linux/utime.c
diff --git a/include/time.h b/include/time.h
index b04747889a..c0d1ea3315 100644
--- a/include/time.h
+++ b/include/time.h
@@ -211,9 +211,12 @@ libc_hidden_proto (__clock_getres64);
#endif
#if __TIMESIZE == 64
+# define __utime64 __utime
# define __utimes64 __utimes
# define __utimensat64 __utimensat
#else
+extern int __utime64 (const char *file, const struct __utimbuf64 *times);
+libc_hidden_proto (__utime64)
extern int __utimes64 (const char *file, const struct __timeval64 tvp[2]);
libc_hidden_proto (__utimes64)
extern int __utimensat64 (int fd, const char *file,
diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
index 5d65ed23e0..e40f993495 100644
--- a/sysdeps/unix/sysv/linux/syscalls.list
+++ b/sysdeps/unix/sysv/linux/syscalls.list
@@ -65,7 +65,6 @@ swapon - swapon i:si __swapon swapon
swapoff - swapoff i:s __swapoff swapoff
unshare EXTRA unshare i:i unshare
uselib EXTRA uselib i:s __compat_uselib uselib@GLIBC_2.0:GLIBC_2.23
-utime - utime i:sP utime
chown - chown i:sii __libc_chown __chown chown
diff --git a/sysdeps/unix/sysv/linux/utime.c b/sysdeps/unix/sysv/linux/utime.c
new file mode 100644
index 0000000000..75ee0745ec
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/utime.c
@@ -0,0 +1,56 @@
+/* utime -- Change access and modification times of file. Linux version.
+ Copyright (C) 1991-2020 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <utime.h>
+#include <time.h>
+
+int
+__utime64 (const char *file, const struct __utimbuf64 *times)
+{
+ struct __timespec64 ts64[2];
+
+ if (times)
+ {
+ ts64[0].tv_sec = times->actime;
+ ts64[0].tv_nsec = 0LL;
+ ts64[1].tv_sec = times->modtime;
+ ts64[1].tv_nsec = 0LL;
+ }
+
+ return __utimensat64_helper (0, file, times ? ts64 : NULL, 0);
+}
+
+#if __TIMESIZE != 64
+libc_hidden_def (__utime64)
+
+int
+__utime (const char *file, const struct utimbuf *times)
+{
+ struct __utimbuf64 utb64;
+
+ if (times)
+ {
+ utb64.actime = (__time64_t) times->actime;
+ utb64.modtime = (__time64_t) times->modtime;
+ }
+
+ return __utime64 (file, times ? &utb64 : NULL);
+}
+#endif
+strong_alias (__utime, utime)
+libc_hidden_def (utime)
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
@ 2020-02-07 20:52 ` Alistair Francis
2020-02-20 14:53 ` Adhemerval Zanella
1 sibling, 0 replies; 13+ messages in thread
From: Alistair Francis @ 2020-02-07 20:52 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On Fri, Feb 7, 2020 at 5:00 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> This type is a glibc's "internal" type to store file's access and modification
> times in __time64_t rather than __time_t, which makes it Y2038-proof.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/time.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 389355a5a5..e38f5e32e6 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -109,6 +109,18 @@ struct __timeval64
> };
> #endif
>
> +#if __TIMESIZE == 64
> +# define __utimbuf64 utimbuf
> +#else
> +/* The glibc Y2038-proof struct __utimbuf64 structure for file's access
> + and modification time values. */
> +struct __utimbuf64
> +{
> + __time64_t actime; /* Access time. */
> + __time64_t modtime; /* Modification time. */
> +};
> +#endif
> +
> #if __TIMESIZE == 64
> # define __itimerval64 itimerval
> #else
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time
2020-02-07 13:00 [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
` (2 preceding siblings ...)
2020-02-07 13:00 ` [PATCH 3/3] y2038: linux: Provide __utime64 implementation Lukasz Majewski
@ 2020-02-14 8:56 ` Lukasz Majewski
3 siblings, 0 replies; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-14 8:56 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]
On Fri, 7 Feb 2020 14:00:06 +0100
Lukasz Majewski <lukma@denx.de> wrote:
> This patch series provides __utime64, __utimes64 and struct
> __utimbuf64.
>
> The branch with integrated Y2038 support:
> https://github.com/lmajewski/y2038_glibc/commits/glibc_utime_conversion_v1
>
> Lukasz Majewski (3):
> y2038: Introduce struct __utimbuf64 - new internal glibc type
> y2038: linux: Provide __utimes64 implementation
> y2038: linux: Provide __utime64 implementation
Gentle ping on this series.
>
> include/time.h | 18 +++++++++
> sysdeps/unix/sysv/linux/syscalls.list | 1 -
> sysdeps/unix/sysv/linux/utime.c | 56
> +++++++++++++++++++++++++++ sysdeps/unix/sysv/linux/utimes.c |
> 37 ++++++++++++------ 4 files changed, 100 insertions(+), 12
> deletions(-) create mode 100644 sysdeps/unix/sysv/linux/utime.c
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
2020-02-07 20:52 ` Alistair Francis
@ 2020-02-20 14:53 ` Adhemerval Zanella
1 sibling, 0 replies; 13+ messages in thread
From: Adhemerval Zanella @ 2020-02-20 14:53 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab
On 07/02/2020 10:00, Lukasz Majewski wrote:
> This type is a glibc's "internal" type to store file's access and modification
> times in __time64_t rather than __time_t, which makes it Y2038-proof.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> include/time.h | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/include/time.h b/include/time.h
> index 389355a5a5..e38f5e32e6 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -109,6 +109,18 @@ struct __timeval64
> };
> #endif
>
> +#if __TIMESIZE == 64
> +# define __utimbuf64 utimbuf
> +#else
> +/* The glibc Y2038-proof struct __utimbuf64 structure for file's access
> + and modification time values. */
> +struct __utimbuf64
> +{
> + __time64_t actime; /* Access time. */
> + __time64_t modtime; /* Modification time. */
> +};
> +#endif
> +
> #if __TIMESIZE == 64
> # define __itimerval64 itimerval
> #else
>
Ok.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] y2038: linux: Provide __utimes64 implementation
2020-02-07 13:00 ` [PATCH 2/3] y2038: linux: Provide __utimes64 implementation Lukasz Majewski
@ 2020-02-20 14:53 ` Adhemerval Zanella
2020-02-20 15:23 ` Lukasz Majewski
0 siblings, 1 reply; 13+ messages in thread
From: Adhemerval Zanella @ 2020-02-20 14:53 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab
On 07/02/2020 10:00, Lukasz Majewski wrote:
> This patch provides new __utimes64 explicit 64 bit function for setting file's
> 64 bit attributes for access and modification time.
>
> Internally, the __utimensat64_helper function is used. This patch is necessary
> for having architectures with __WORDSIZE == 32 Y2038 safe.
>
> Moreover, a 32 bit version - __utimes has been refactored to internally use
> __utimes64.
>
> The __utimes is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversion of struct
> timeval to 64 bit struct __timeval64.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
> https://github.com/lmajewski/meta-y2038 and run tests:
> https://github.com/lmajewski/y2038-tests/commits/master
>
> Above tests were performed with Y2038 redirection applied as well as without
> to test proper usage of both __utimes64 and __utimes.
LGTM with some smalls changes below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> include/time.h | 3 +++
> sysdeps/unix/sysv/linux/utimes.c | 37 ++++++++++++++++++++++----------
> 2 files changed, 29 insertions(+), 11 deletions(-)
>
> diff --git a/include/time.h b/include/time.h
> index e38f5e32e6..b04747889a 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -211,8 +211,11 @@ libc_hidden_proto (__clock_getres64);
> #endif
>
> #if __TIMESIZE == 64
> +# define __utimes64 __utimes
> # define __utimensat64 __utimensat
> #else
> +extern int __utimes64 (const char *file, const struct __timeval64 tvp[2]);
> +libc_hidden_proto (__utimes64)
> extern int __utimensat64 (int fd, const char *file,
> const struct __timespec64 tsp[2], int flags);
> libc_hidden_proto (__utimensat64);
Ok.
> diff --git a/sysdeps/unix/sysv/linux/utimes.c b/sysdeps/unix/sysv/linux/utimes.c
> index 121d883469..09c4e56f18 100644
> --- a/sysdeps/unix/sysv/linux/utimes.c
> +++ b/sysdeps/unix/sysv/linux/utimes.c
> @@ -16,22 +16,37 @@
> License along with the GNU C Library; if not, see
> <https://www.gnu.org/licenses/>. */
>
> -#include <errno.h>
> -#include <stddef.h>
> -#include <utime.h>
> -#include <sys/time.h>
> -#include <sysdep.h>
> +#include <time.h>
>
> +int
> +__utimes64 (const char *file, const struct __timeval64 tvp[2])
> +{
> + struct __timespec64 ts64[2];
> +
> + if (tvp)
No implicit checks.
> + {
> + ts64[0] = timeval64_to_timespec64 (tvp[0]);
> + ts64[1] = timeval64_to_timespec64 (tvp[1]);
> + }
> +
> + return __utimensat64_helper (0, file, tvp ? ts64 : NULL, 0);
> +}
Ok.
>
> -/* Consider moving to syscalls.list. */
> +#if __TIMESIZE != 64
> +libc_hidden_def (__utimes64)
>
> -/* Change the access time of FILE to TVP[0] and
> - the modification time of FILE to TVP[1]. */
> int
> __utimes (const char *file, const struct timeval tvp[2])
> {
> - /* Avoid implicit array coercion in syscall macros. */
> - return INLINE_SYSCALL (utimes, 2, file, &tvp[0]);
> -}
> + struct __timeval64 tv64[2];
>
> + if (tvp)
No implicit checks.
> + {
> + tv64[0] = valid_timeval_to_timeval64 (tvp[0]);
> + tv64[1] = valid_timeval_to_timeval64 (tvp[1]);
> + }
> +
> + return __utimes64 (file, tvp ? tv64 : NULL);
> +}
> +#endif
> weak_alias (__utimes, utimes)
>
Ok.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] y2038: linux: Provide __utime64 implementation
2020-02-07 13:00 ` [PATCH 3/3] y2038: linux: Provide __utime64 implementation Lukasz Majewski
@ 2020-02-20 14:53 ` Adhemerval Zanella
2020-02-20 15:36 ` Lukasz Majewski
0 siblings, 1 reply; 13+ messages in thread
From: Adhemerval Zanella @ 2020-02-20 14:53 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert
Cc: Alistair Francis, Alistair Francis, GNU C Library,
Siddhesh Poyarekar, Florian Weimer, Florian Weimer, Zack Weinberg,
Carlos O'Donell, Andreas Schwab
On 07/02/2020 10:00, Lukasz Majewski wrote:
> This patch replaces auto generated wrapper (as described in
> sysdeps/unix/sysv/linux/syscalls.list) for utime with one which adds extra
> support for setting file's access and modification 64 bit time on machines
> with __TIMESIZE != 64.
>
> Internally, the __utimensat_time64 helper function is used. This patch is
> necessary for having architectures with __WORDSIZE == 32 && __TIMESIZE != 64
> Y2038 safe.
>
> Moreover, a 32 bit version - __utime has been refactored to internally use
> __utime64.
> The __utime is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary conversion between struct
> utimbuf and struct __utimbuf64.
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
>
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
> https://github.com/lmajewski/meta-y2038 and run tests:
> https://github.com/lmajewski/y2038-tests/commits/master
>
> Above tests were performed with Y2038 redirection applied as well as
> without to test proper usage of both __utime64 and __utime.
LGTM with some smalls changes below.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> include/time.h | 3 ++
> sysdeps/unix/sysv/linux/syscalls.list | 1 -
> sysdeps/unix/sysv/linux/utime.c | 56 +++++++++++++++++++++++++++
> 3 files changed, 59 insertions(+), 1 deletion(-)
> create mode 100644 sysdeps/unix/sysv/linux/utime.c
>
> diff --git a/include/time.h b/include/time.h
> index b04747889a..c0d1ea3315 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -211,9 +211,12 @@ libc_hidden_proto (__clock_getres64);
> #endif
>
> #if __TIMESIZE == 64
> +# define __utime64 __utime
> # define __utimes64 __utimes
> # define __utimensat64 __utimensat
> #else
> +extern int __utime64 (const char *file, const struct __utimbuf64 *times);
> +libc_hidden_proto (__utime64)
> extern int __utimes64 (const char *file, const struct __timeval64 tvp[2]);
> libc_hidden_proto (__utimes64)
> extern int __utimensat64 (int fd, const char *file,
Ok.
> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index 5d65ed23e0..e40f993495 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -65,7 +65,6 @@ swapon - swapon i:si __swapon swapon
> swapoff - swapoff i:s __swapoff swapoff
> unshare EXTRA unshare i:i unshare
> uselib EXTRA uselib i:s __compat_uselib uselib@GLIBC_2.0:GLIBC_2.23
> -utime - utime i:sP utime
>
> chown - chown i:sii __libc_chown __chown chown
>
Ok.
> diff --git a/sysdeps/unix/sysv/linux/utime.c b/sysdeps/unix/sysv/linux/utime.c
> new file mode 100644
> index 0000000000..75ee0745ec
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/utime.c
> @@ -0,0 +1,56 @@
> +/* utime -- Change access and modification times of file. Linux version.
> + Copyright (C) 1991-2020 Free Software Foundation, Inc.
I think it should be only 2020, since it is a new implementation.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +#include <utime.h>
> +#include <time.h>
> +
> +int
> +__utime64 (const char *file, const struct __utimbuf64 *times)
> +{
> + struct __timespec64 ts64[2];
> +
> + if (times)
No implicit checks.
> + {
> + ts64[0].tv_sec = times->actime;
> + ts64[0].tv_nsec = 0LL;
> + ts64[1].tv_sec = times->modtime;
> + ts64[1].tv_nsec = 0LL;
> + }
Should we use type modifiers here?
> +
> + return __utimensat64_helper (0, file, times ? ts64 : NULL, 0);
> +}
Ok.
> +
> +#if __TIMESIZE != 64
> +libc_hidden_def (__utime64)
> +
> +int
> +__utime (const char *file, const struct utimbuf *times)
> +{
> + struct __utimbuf64 utb64;
> +
> + if (times)
No implicit checks.
> + {
> + utb64.actime = (__time64_t) times->actime;
> + utb64.modtime = (__time64_t) times->modtime;
Do we need to explicit cast here?
> + }
> +
> + return __utime64 (file, times ? &utb64 : NULL);
> +}
> +#endif
> +strong_alias (__utime, utime)
> +libc_hidden_def (utime)
>
Ok.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] y2038: linux: Provide __utimes64 implementation
2020-02-20 14:53 ` Adhemerval Zanella
@ 2020-02-20 15:23 ` Lukasz Majewski
2020-02-20 17:00 ` Adhemerval Zanella
0 siblings, 1 reply; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-20 15:23 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 4617 bytes --]
Hi Adhemerval,
> On 07/02/2020 10:00, Lukasz Majewski wrote:
> > This patch provides new __utimes64 explicit 64 bit function for
> > setting file's 64 bit attributes for access and modification time.
> >
> > Internally, the __utimensat64_helper function is used. This patch
> > is necessary for having architectures with __WORDSIZE == 32 Y2038
> > safe.
> >
> > Moreover, a 32 bit version - __utimes has been refactored to
> > internally use __utimes64.
> >
> > The __utimes is now supposed to be used on systems still supporting
> > 32 bit time (__TIMESIZE != 64) - hence the necessary conversion of
> > struct timeval to 64 bit struct __timeval64.
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> >
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> > https://github.com/lmajewski/meta-y2038 and run tests:
> > https://github.com/lmajewski/y2038-tests/commits/master
> >
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test proper usage of both __utimes64 and __utimes.
>
> LGTM with some smalls changes below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > include/time.h | 3 +++
> > sysdeps/unix/sysv/linux/utimes.c | 37
> > ++++++++++++++++++++++---------- 2 files changed, 29 insertions(+),
> > 11 deletions(-)
> >
> > diff --git a/include/time.h b/include/time.h
> > index e38f5e32e6..b04747889a 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -211,8 +211,11 @@ libc_hidden_proto (__clock_getres64);
> > #endif
> >
> > #if __TIMESIZE == 64
> > +# define __utimes64 __utimes
> > # define __utimensat64 __utimensat
> > #else
> > +extern int __utimes64 (const char *file, const struct __timeval64
> > tvp[2]); +libc_hidden_proto (__utimes64)
> > extern int __utimensat64 (int fd, const char *file,
> > const struct __timespec64 tsp[2], int
> > flags); libc_hidden_proto (__utimensat64);
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/utimes.c
> > b/sysdeps/unix/sysv/linux/utimes.c index 121d883469..09c4e56f18
> > 100644 --- a/sysdeps/unix/sysv/linux/utimes.c
> > +++ b/sysdeps/unix/sysv/linux/utimes.c
> > @@ -16,22 +16,37 @@
> > License along with the GNU C Library; if not, see
> > <https://www.gnu.org/licenses/>. */
> >
> > -#include <errno.h>
> > -#include <stddef.h>
> > -#include <utime.h>
> > -#include <sys/time.h>
> > -#include <sysdep.h>
> > +#include <time.h>
> >
> > +int
> > +__utimes64 (const char *file, const struct __timeval64 tvp[2])
> > +{
> > + struct __timespec64 ts64[2];
> > +
> > + if (tvp)
>
> No implicit checks.
The documentation [1] and [2] explicitly says that times (here tvp) can
be NULL:
"If times is NULL, then the access and modification times of the file
are set to the current time. "
Hence, it is perfectly valid to pass the NULL to the
__utimensat64_helper(). Without this check we will segfault earlier
(before we reach proper syscall) and introduce regression in glibc.
Links:
[1] - https://linux.die.net/man/2/utimes
[2] - https://www.unix.com/man-page/linux/2/utimes/
>
> > + {
> > + ts64[0] = timeval64_to_timespec64 (tvp[0]);
> > + ts64[1] = timeval64_to_timespec64 (tvp[1]);
> > + }
> > +
> > + return __utimensat64_helper (0, file, tvp ? ts64 : NULL, 0);
> > +}
>
> Ok.
>
> >
> > -/* Consider moving to syscalls.list. */
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__utimes64)
> >
> > -/* Change the access time of FILE to TVP[0] and
> > - the modification time of FILE to TVP[1]. */
> > int
> > __utimes (const char *file, const struct timeval tvp[2])
> > {
> > - /* Avoid implicit array coercion in syscall macros. */
> > - return INLINE_SYSCALL (utimes, 2, file, &tvp[0]);
> > -}
> > + struct __timeval64 tv64[2];
> >
> > + if (tvp)
>
> No implicit checks.
Please consider the above comment.
>
> > + {
> > + tv64[0] = valid_timeval_to_timeval64 (tvp[0]);
> > + tv64[1] = valid_timeval_to_timeval64 (tvp[1]);
> > + }
> > +
> > + return __utimes64 (file, tvp ? tv64 : NULL);
> > +}
> > +#endif
> > weak_alias (__utimes, utimes)
> >
>
> Ok.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] y2038: linux: Provide __utime64 implementation
2020-02-20 14:53 ` Adhemerval Zanella
@ 2020-02-20 15:36 ` Lukasz Majewski
0 siblings, 0 replies; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-20 15:36 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 6234 bytes --]
Hi Adhemerval,
> On 07/02/2020 10:00, Lukasz Majewski wrote:
> > This patch replaces auto generated wrapper (as described in
> > sysdeps/unix/sysv/linux/syscalls.list) for utime with one which
> > adds extra support for setting file's access and modification 64
> > bit time on machines with __TIMESIZE != 64.
> >
> > Internally, the __utimensat_time64 helper function is used. This
> > patch is necessary for having architectures with __WORDSIZE == 32
> > && __TIMESIZE != 64 Y2038 safe.
> >
> > Moreover, a 32 bit version - __utime has been refactored to
> > internally use __utime64.
> > The __utime is now supposed to be used on systems still supporting
> > 32 bit time (__TIMESIZE != 64) - hence the necessary conversion
> > between struct utimbuf and struct __utimbuf64.
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> >
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> > https://github.com/lmajewski/meta-y2038 and run tests:
> > https://github.com/lmajewski/y2038-tests/commits/master
> >
> > Above tests were performed with Y2038 redirection applied as well as
> > without to test proper usage of both __utime64 and __utime.
>
> LGTM with some smalls changes below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > include/time.h | 3 ++
> > sysdeps/unix/sysv/linux/syscalls.list | 1 -
> > sysdeps/unix/sysv/linux/utime.c | 56
> > +++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 1
> > deletion(-) create mode 100644 sysdeps/unix/sysv/linux/utime.c
> >
> > diff --git a/include/time.h b/include/time.h
> > index b04747889a..c0d1ea3315 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -211,9 +211,12 @@ libc_hidden_proto (__clock_getres64);
> > #endif
> >
> > #if __TIMESIZE == 64
> > +# define __utime64 __utime
> > # define __utimes64 __utimes
> > # define __utimensat64 __utimensat
> > #else
> > +extern int __utime64 (const char *file, const struct __utimbuf64
> > *times); +libc_hidden_proto (__utime64)
> > extern int __utimes64 (const char *file, const struct __timeval64
> > tvp[2]); libc_hidden_proto (__utimes64)
> > extern int __utimensat64 (int fd, const char *file,
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/syscalls.list
> > b/sysdeps/unix/sysv/linux/syscalls.list index
> > 5d65ed23e0..e40f993495 100644 ---
> > a/sysdeps/unix/sysv/linux/syscalls.list +++
> > b/sysdeps/unix/sysv/linux/syscalls.list @@ -65,7 +65,6 @@
> > swapon - swapon i:si
> > __swapon swapon swapoff -
> > swapoff i:s __swapoff swapoff
> > unshare EXTRA unshare
> > i:i unshare uselib EXTRA uselib
> > i:s __compat_uselib
> > uselib@GLIBC_2.0:GLIBC_2.23 -utime -
> > utime i:sP utime chown
> > - chown i:sii __libc_chown
> > __chown chown
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/utime.c
> > b/sysdeps/unix/sysv/linux/utime.c new file mode 100644
> > index 0000000000..75ee0745ec
> > --- /dev/null
> > +++ b/sysdeps/unix/sysv/linux/utime.c
> > @@ -0,0 +1,56 @@
> > +/* utime -- Change access and modification times of file. Linux
> > version.
> > + Copyright (C) 1991-2020 Free Software Foundation, Inc.
>
> I think it should be only 2020, since it is a new implementation.
Ok.
>
> > + This file is part of the GNU C Library.
> > +
> > + The GNU C Library is free software; you can redistribute it
> > and/or
> > + modify it under the terms of the GNU Lesser General Public
> > + License as published by the Free Software Foundation; either
> > + version 2.1 of the License, or (at your option) any later
> > version. +
> > + The GNU C Library is distributed in the hope that it will be
> > useful,
> > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > GNU
> > + Lesser General Public License for more details.
> > +
> > + You should have received a copy of the GNU Lesser General Public
> > + License along with the GNU C Library; if not, see
> > + <https://www.gnu.org/licenses/>. */
> > +
> > +#include <utime.h>
> > +#include <time.h>
> > +
> > +int
> > +__utime64 (const char *file, const struct __utimbuf64 *times)
> > +{
> > + struct __timespec64 ts64[2];
> > +
> > + if (times)
>
> No implicit checks.
Please consider my reply to __utimes64 patch.
>
> > + {
> > + ts64[0].tv_sec = times->actime;
> > + ts64[0].tv_nsec = 0LL;
> > + ts64[1].tv_sec = times->modtime;
> > + ts64[1].tv_nsec = 0LL;
> > + }
>
> Should we use type modifiers here?
the __utime64 has following parameters: (const char *file, const struct
__utimbuf64 *times)
Hence, the times has __time64_t actime and modtime members and casting
them to __time64_t tv.sec shall be safe.
>
> > +
> > + return __utimensat64_helper (0, file, times ? ts64 : NULL, 0);
> > +}
>
> Ok.
>
> > +
> > +#if __TIMESIZE != 64
> > +libc_hidden_def (__utime64)
> > +
> > +int
> > +__utime (const char *file, const struct utimbuf *times)
> > +{
> > + struct __utimbuf64 utb64;
> > +
> > + if (times)
>
> No implicit checks.
>
> > + {
> > + utb64.actime = (__time64_t) times->actime;
> > + utb64.modtime = (__time64_t) times->modtime;
>
> Do we need to explicit cast here?
We could get away with not having them (as we cast from smaller range
integer to larger one).
However, the __utime's *times parameter points to struct utimbuf, which
atime and modtime fields are __time_t (32 bit on ARM32).
I've add those casts to emphasize that we do convert to 64 bit types.
>
> > + }
> > +
> > + return __utime64 (file, times ? &utb64 : NULL);
> > +}
> > +#endif
> > +strong_alias (__utime, utime)
> > +libc_hidden_def (utime)
> >
>
> Ok.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] y2038: linux: Provide __utimes64 implementation
2020-02-20 15:23 ` Lukasz Majewski
@ 2020-02-20 17:00 ` Adhemerval Zanella
2020-02-20 22:25 ` Lukasz Majewski
0 siblings, 1 reply; 13+ messages in thread
From: Adhemerval Zanella @ 2020-02-20 17:00 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
On 20/02/2020 12:23, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 07/02/2020 10:00, Lukasz Majewski wrote:
>>> This patch provides new __utimes64 explicit 64 bit function for
>>> setting file's 64 bit attributes for access and modification time.
>>>
>>> Internally, the __utimensat64_helper function is used. This patch
>>> is necessary for having architectures with __WORDSIZE == 32 Y2038
>>> safe.
>>>
>>> Moreover, a 32 bit version - __utimes has been refactored to
>>> internally use __utimes64.
>>>
>>> The __utimes is now supposed to be used on systems still supporting
>>> 32 bit time (__TIMESIZE != 64) - hence the necessary conversion of
>>> struct timeval to 64 bit struct __timeval64.
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>>
>>> Run-time tests:
>>> - Run specific tests on ARM/x86 32bit systems (qemu):
>>> https://github.com/lmajewski/meta-y2038 and run tests:
>>> https://github.com/lmajewski/y2038-tests/commits/master
>>>
>>> Above tests were performed with Y2038 redirection applied as well
>>> as without to test proper usage of both __utimes64 and __utimes.
>>
>> LGTM with some smalls changes below.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>
>>> ---
>>> include/time.h | 3 +++
>>> sysdeps/unix/sysv/linux/utimes.c | 37
>>> ++++++++++++++++++++++---------- 2 files changed, 29 insertions(+),
>>> 11 deletions(-)
>>>
>>> diff --git a/include/time.h b/include/time.h
>>> index e38f5e32e6..b04747889a 100644
>>> --- a/include/time.h
>>> +++ b/include/time.h
>>> @@ -211,8 +211,11 @@ libc_hidden_proto (__clock_getres64);
>>> #endif
>>>
>>> #if __TIMESIZE == 64
>>> +# define __utimes64 __utimes
>>> # define __utimensat64 __utimensat
>>> #else
>>> +extern int __utimes64 (const char *file, const struct __timeval64
>>> tvp[2]); +libc_hidden_proto (__utimes64)
>>> extern int __utimensat64 (int fd, const char *file,
>>> const struct __timespec64 tsp[2], int
>>> flags); libc_hidden_proto (__utimensat64);
>>
>> Ok.
>>
>>> diff --git a/sysdeps/unix/sysv/linux/utimes.c
>>> b/sysdeps/unix/sysv/linux/utimes.c index 121d883469..09c4e56f18
>>> 100644 --- a/sysdeps/unix/sysv/linux/utimes.c
>>> +++ b/sysdeps/unix/sysv/linux/utimes.c
>>> @@ -16,22 +16,37 @@
>>> License along with the GNU C Library; if not, see
>>> <https://www.gnu.org/licenses/>. */
>>>
>>> -#include <errno.h>
>>> -#include <stddef.h>
>>> -#include <utime.h>
>>> -#include <sys/time.h>
>>> -#include <sysdep.h>
>>> +#include <time.h>
>>>
>>> +int
>>> +__utimes64 (const char *file, const struct __timeval64 tvp[2])
>>> +{
>>> + struct __timespec64 ts64[2];
>>> +
>>> + if (tvp)
>>
>> No implicit checks.
>
> The documentation [1] and [2] explicitly says that times (here tvp) can
> be NULL:
> "If times is NULL, then the access and modification times of the file
> are set to the current time. "
>
> Hence, it is perfectly valid to pass the NULL to the
> __utimensat64_helper(). Without this check we will segfault earlier
> (before we reach proper syscall) and introduce regression in glibc.
I meant to use:
if (tvp != NULL)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] y2038: linux: Provide __utimes64 implementation
2020-02-20 17:00 ` Adhemerval Zanella
@ 2020-02-20 22:25 ` Lukasz Majewski
0 siblings, 0 replies; 13+ messages in thread
From: Lukasz Majewski @ 2020-02-20 22:25 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: Joseph Myers, Paul Eggert, Alistair Francis, Alistair Francis,
GNU C Library, Siddhesh Poyarekar, Florian Weimer, Florian Weimer,
Zack Weinberg, Carlos O'Donell, Andreas Schwab
[-- Attachment #1: Type: text/plain, Size: 3869 bytes --]
Hi Adhemerval,
> On 20/02/2020 12:23, Lukasz Majewski wrote:
> > Hi Adhemerval,
> >
> >> On 07/02/2020 10:00, Lukasz Majewski wrote:
> >>> This patch provides new __utimes64 explicit 64 bit function for
> >>> setting file's 64 bit attributes for access and modification time.
> >>>
> >>> Internally, the __utimensat64_helper function is used. This patch
> >>> is necessary for having architectures with __WORDSIZE == 32 Y2038
> >>> safe.
> >>>
> >>> Moreover, a 32 bit version - __utimes has been refactored to
> >>> internally use __utimes64.
> >>>
> >>> The __utimes is now supposed to be used on systems still
> >>> supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> >>> conversion of struct timeval to 64 bit struct __timeval64.
> >>>
> >>> Build tests:
> >>> ./src/scripts/build-many-glibcs.py glibcs
> >>>
> >>> Run-time tests:
> >>> - Run specific tests on ARM/x86 32bit systems (qemu):
> >>> https://github.com/lmajewski/meta-y2038 and run tests:
> >>> https://github.com/lmajewski/y2038-tests/commits/master
> >>>
> >>> Above tests were performed with Y2038 redirection applied as well
> >>> as without to test proper usage of both __utimes64 and __utimes.
> >>>
> >>
> >> LGTM with some smalls changes below.
> >>
> >> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> >>
> >>> ---
> >>> include/time.h | 3 +++
> >>> sysdeps/unix/sysv/linux/utimes.c | 37
> >>> ++++++++++++++++++++++---------- 2 files changed, 29
> >>> insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/include/time.h b/include/time.h
> >>> index e38f5e32e6..b04747889a 100644
> >>> --- a/include/time.h
> >>> +++ b/include/time.h
> >>> @@ -211,8 +211,11 @@ libc_hidden_proto (__clock_getres64);
> >>> #endif
> >>>
> >>> #if __TIMESIZE == 64
> >>> +# define __utimes64 __utimes
> >>> # define __utimensat64 __utimensat
> >>> #else
> >>> +extern int __utimes64 (const char *file, const struct __timeval64
> >>> tvp[2]); +libc_hidden_proto (__utimes64)
> >>> extern int __utimensat64 (int fd, const char *file,
> >>> const struct __timespec64 tsp[2], int
> >>> flags); libc_hidden_proto (__utimensat64);
> >>
> >> Ok.
> >>
> >>> diff --git a/sysdeps/unix/sysv/linux/utimes.c
> >>> b/sysdeps/unix/sysv/linux/utimes.c index 121d883469..09c4e56f18
> >>> 100644 --- a/sysdeps/unix/sysv/linux/utimes.c
> >>> +++ b/sysdeps/unix/sysv/linux/utimes.c
> >>> @@ -16,22 +16,37 @@
> >>> License along with the GNU C Library; if not, see
> >>> <https://www.gnu.org/licenses/>. */
> >>>
> >>> -#include <errno.h>
> >>> -#include <stddef.h>
> >>> -#include <utime.h>
> >>> -#include <sys/time.h>
> >>> -#include <sysdep.h>
> >>> +#include <time.h>
> >>>
> >>> +int
> >>> +__utimes64 (const char *file, const struct __timeval64 tvp[2])
> >>> +{
> >>> + struct __timespec64 ts64[2];
> >>> +
> >>> + if (tvp)
> >>
> >> No implicit checks.
> >
> > The documentation [1] and [2] explicitly says that times (here tvp)
> > can be NULL:
> > "If times is NULL, then the access and modification times of the
> > file are set to the current time. "
> >
> > Hence, it is perfectly valid to pass the NULL to the
> > __utimensat64_helper(). Without this check we will segfault earlier
> > (before we reach proper syscall) and introduce regression in glibc.
> >
>
> I meant to use:
>
> if (tvp != NULL)
Ach ... :-) Thanks for the clarification.
I thought that this check is not needed at all. My misunderstanding :-)
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-02-20 22:26 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 13:00 [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
2020-02-07 13:00 ` [PATCH 1/3] y2038: Introduce struct __utimbuf64 - new internal glibc type Lukasz Majewski
2020-02-07 20:52 ` Alistair Francis
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-07 13:00 ` [PATCH 2/3] y2038: linux: Provide __utimes64 implementation Lukasz Majewski
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-20 15:23 ` Lukasz Majewski
2020-02-20 17:00 ` Adhemerval Zanella
2020-02-20 22:25 ` Lukasz Majewski
2020-02-07 13:00 ` [PATCH 3/3] y2038: linux: Provide __utime64 implementation Lukasz Majewski
2020-02-20 14:53 ` Adhemerval Zanella
2020-02-20 15:36 ` Lukasz Majewski
2020-02-14 8:56 ` [PATCH 0/3] y2038: Refactor utime and utimes to support 64 bit time Lukasz Majewski
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 read-only IMAP folder(s) and NNTP newsgroup(s).