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 > > > --- > > 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 > > + . */ > > + > > +#include > > +#include > > + > > +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