Message ID | 20200207130009.19396-4-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | y2038: Refactor utime and utimes to support 64 bit time | expand |
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.
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
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)