From: Alistair Francis <alistair23@gmail.com>
To: Joseph Myers <joseph@codesourcery.com>
Cc: Alistair Francis <alistair.francis@wdc.com>,
GNU C Library <libc-alpha@sourceware.org>,
Arnd Bergmann <arnd@arndb.de>,
Adhemerval Zanella <adhemerval.zanella@linaro.org>,
Florian Weimer <fweimer@redhat.com>,
Palmer Dabbelt <palmer@sifive.com>,
macro@wdc.com, Zong Li <zongbox@gmail.com>
Subject: Re: [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 if avaliable
Date: Thu, 15 Aug 2019 20:52:00 -0000 [thread overview]
Message-ID: <CAKmqyKMGXkeUaGy=Yu+=RWq0wSNVE1pTy-_3tEA44ikVNVmUZA@mail.gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.21.1908151938180.31825@digraph.polyomino.org.uk>
On Thu, Aug 15, 2019 at 12:46 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 15 Aug 2019, Alistair Francis wrote:
>
> > Is this the type of layout you are looking for? (untested, just a general idea)
>
> I'm not convinced making timespec_get into a wrapper round another
> function is a good thing.
>
> You need to end up, in the __TIMESIZE == 32 case, with two functions that
> have the public timespec_get semantics - not two internal functions and a
> single wrapper - in preparation for when we support _TIME_BITS == 64. So
> I'd expect defining __timespec_get64 as a function with the public
> semantics (not as an internal function that only calls the syscall), and
> conditionally defining timespec_get as a thin wrapper in the case of
> 32-bit time_t, and having a #define of __timespec_get64 to timespec_get in
> an internal header in the case of 64-bit time_t.
Ok, so more like this?
#if __TIMESIZE == 64
# define timespec_get __timespec_get64
#else
# define timespec_get __timespec_get32
#endif
/* Set TS to calendar time based in time base BASE. */
int
__timespec_get64 (struct timespec *ts, int base)
{
switch (base)
{
int res;
INTERNAL_SYSCALL_DECL (err);
case TIME_UTC:
#ifdef __ASSUME_TIME64_SYSCALLS
res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
#else
# ifdef __NR_clock_gettime64
res = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
# endif /* __NR_clock_gettime64 */
struct timespec ts32;
if (! in_time_t_range (ts->tv_sec))
{
__set_errno (EOVERFLOW);
return -1;
}
valid_timespec64_to_timespec (ts, &ts32);
res = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
{
ts->tv_sec = ts32.tv_sec;
ts->tv_nsec = ts32.tv_nsec;
}
#endif
if (INTERNAL_SYSCALL_ERROR_P (res, err))
return 0;
break;
default:
return 0;
}
return base;
}
#if __TIMESIZE != 64
int
__timespec_get32 (struct timespec *ts, int base)
{
struct __timespec64 ts64;
ret = __timespec_get64 (ts64, base);
if (res == 0 || !INTERNAL_SYSCALL_ERROR_P (res, err))
{
ts->tv_sec = ts64.tv_sec;
ts->tv_nsec = ts64.tv_nsec;
}
return ret;
}
#endif
>
> > int
> > __timespec_get64 (struct __timespec64 *ts, int base)
> > {
> > #ifdef __ASSUME_TIME64_SYSCALLS
> > return INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> > #else
>
> You need
>
> # ifndef __NR_clock_gettime64
> # define __NR_clock_gettime64 __NR_clock_gettime
> # endif
>
> here, because 64-bit platforms define __ASSUME_TIME64_SYSCALLS but with
> unsuffixed syscall names.
The kernel defines 64 suffixed syscalls, is this required because
older kernels don't do this?
>
> > long int ret;
> > # ifdef __NR_clock_gettime64
> > ret = INTERNAL_VSYSCALL (clock_gettime64, err, 2, CLOCK_REALTIME, ts);
> >
> > if (ret == 0 || errno != ENOSYS)
> > {
> > return ret;
> > }
>
> You need to check INTERNAL_SYSCALL_ERRNO (...) not errno; errno isn't set
> by INTERNAL_*, only by INLINE_*.
Fixed! Thanks
>
> No braces when only a single statement is inside them.
>
> > ret = INTERNAL_VSYSCALL (clock_gettime, err, 2, CLOCK_REALTIME, &ts32);
> >
> > if ((ret == 0 || errno != ENOSYS) && ts)
>
> No need to consider ENOSYS here. NULL ts isn't valid so no need to check
> for it being non-NULL.
Fixed
>
> > /* Set TS to calendar time based in time base BASE. */
> > int
> > timespec_get (struct timespec *ts, int base)
> > {
> > switch (base)
> > {
> > int res;
> > INTERNAL_SYSCALL_DECL (err);
> > case TIME_UTC:
> > res = __timespec_get (ts, base);
> > if (INTERNAL_SYSCALL_ERROR_P (res, err))
> > return 0;
>
> This wrapper is using an uninitialized variable err.
Good point, fixed.
Alistair
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
next prev parent reply other threads:[~2019-08-15 20:52 UTC|newest]
Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-08-10 1:03 [RFC v4 00/24] RISC-V glibc port for the 32-bit Alistair Francis
2019-08-10 1:03 ` [RFC v4 16/24] RISC-V: The ABI implementation " Alistair Francis
2019-08-10 1:03 ` [RFC v4 08/24] sysdeps/stat: Copy the statx struct to stat instead of stat64 Alistair Francis
2019-08-12 20:01 ` Joseph Myers
2019-08-12 22:26 ` Alistair Francis
2019-08-12 23:02 ` Joseph Myers
2019-08-12 23:10 ` Alistair Francis
2019-08-12 23:31 ` Joseph Myers
2019-08-13 20:40 ` Alistair Francis
2019-08-10 1:03 ` [RFC v4 10/24] RISC-V: Use 64-bit time_t and off_t for RV32 and RV64 Alistair Francis
2019-08-10 1:03 ` [RFC v4 09/24] Documentation for the RISC-V 32-bit port Alistair Francis
2019-08-12 20:02 ` Joseph Myers
2019-08-10 1:03 ` [RFC v4 04/24] sysdeps/wait: Use waitid if avaliable Alistair Francis
2019-08-10 1:03 ` [RFC v4 06/24] sysdeps/timespec_get: Use clock_gettime64 " Alistair Francis
2019-08-12 19:46 ` Joseph Myers
2019-08-15 18:03 ` Alistair Francis
2019-08-15 19:46 ` Joseph Myers
2019-08-15 20:52 ` Alistair Francis [this message]
2019-08-15 20:59 ` Joseph Myers
2019-08-15 21:16 ` Alistair Francis
2019-08-15 21:19 ` Joseph Myers
2019-08-15 21:23 ` Alistair Francis
2019-08-15 21:28 ` Joseph Myers
2019-08-15 21:35 ` Alistair Francis
2019-08-10 1:03 ` [RFC v4 01/24] sunrpc/clnt_udp: Ensure total_deadline is initalised Alistair Francis
2019-08-10 1:03 ` [RFC v4 03/24] sysdeps/gettimeofday: Use clock_gettime64 if avaliable Alistair Francis
2019-08-12 17:27 ` Joseph Myers
2019-08-10 1:03 ` [RFC v4 07/24] time: Deprecate struct timezone members Alistair Francis
2019-08-10 1:20 ` Paul Eggert
2019-08-10 1:30 ` Alistair Francis
2019-08-11 13:52 ` Florian Weimer
2019-08-11 16:59 ` Alistair Francis
2019-08-11 17:28 ` Rich Felker
2019-08-12 8:22 ` Florian Weimer
2019-08-12 15:42 ` Zack Weinberg
2019-08-12 20:20 ` Joseph Myers
2019-08-12 21:08 ` Zack Weinberg
2019-08-12 21:26 ` Joseph Myers
2019-08-12 22:13 ` Alistair Francis
2019-08-12 23:16 ` Zack Weinberg
2019-08-13 0:53 ` Paul Eggert
2019-08-13 1:04 ` Zack Weinberg
2019-08-12 22:11 ` Alistair Francis
2019-08-10 1:03 ` [RFC v4 05/24] sysdeps/clock_gettime: Use clock_gettime64 if avaliable Alistair Francis
2019-08-12 19:46 ` Joseph Myers
2019-08-13 23:48 ` Alistair Francis
2019-08-14 16:37 ` Joseph Myers
2019-08-14 18:12 ` Alistair Francis
2019-08-10 1:03 ` [RFC v4 02/24] sysdeps/nanosleep: Use clock_nanosleep_time64 " Alistair Francis
2019-08-12 17:22 ` Joseph Myers
2019-08-14 18:24 ` Alistair Francis
2019-08-14 20:15 ` Joseph Myers
2019-08-14 21:39 ` Stepan Golosunov
2019-08-10 1:03 ` [RFC v4 17/24] RISC-V: Hard float support for the 32 bit Alistair Francis
2019-08-10 1:03 ` [RFC v4 12/24] RISC-V: define __NR_* as __NR_*_time64/64 for 32-bit Alistair Francis
2019-08-10 1:03 ` [RFC v4 13/24] RISC-V: Use 64-bit timespec in clock_gettime vdso calls Alistair Francis
2019-08-10 1:03 ` [RFC v4 15/24] RISC-V: Add path of library directories for RV32 Alistair Francis
2019-08-10 1:04 ` [RFC v4 20/24] RISC-V: Fix llrint and llround missing exceptions on RV32 Alistair Francis
2019-08-10 1:04 ` [RFC v4 21/24] Add RISC-V 32-bit target to build-many-glibcs.py Alistair Francis
2019-08-10 1:04 ` [RFC v4 14/24] RISC-V: Support dynamic loader for the 32-bit Alistair Francis
2019-08-10 1:04 ` [RFC v4 24/24] timerfd_settime: Use 64-bit call if avaliable Alistair Francis
2019-08-10 1:28 ` Alistair Francis
2019-08-20 20:11 ` Maciej W. Rozycki
2019-08-12 20:09 ` Joseph Myers
2019-08-10 1:04 ` [RFC v4 23/24] WIP: syscall.list: Call 64-bit versions of syscalls Alistair Francis
2019-08-14 18:39 ` Alistair Francis
2019-08-14 18:57 ` Florian Weimer
2019-08-15 21:43 ` Alistair Francis
2019-08-19 11:30 ` Florian Weimer
2019-08-10 1:04 ` [RFC v4 22/24] RISC-V: Use 64-bit vdso syscalls for RV32 Alistair Francis
2019-08-10 1:04 ` [RFC v4 18/24] RISC-V: Add ABI lists Alistair Francis
2019-08-10 1:04 ` [RFC v4 19/24] RISC-V: Build Infastructure for the 32-bit Alistair Francis
2019-08-10 1:04 ` [RFC v4 11/24] RISC-V: define __NR_futex as __NR_futex_time64 for 32-bit Alistair Francis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKmqyKMGXkeUaGy=Yu+=RWq0wSNVE1pTy-_3tEA44ikVNVmUZA@mail.gmail.com' \
--to=alistair23@gmail.com \
--cc=adhemerval.zanella@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=arnd@arndb.de \
--cc=fweimer@redhat.com \
--cc=joseph@codesourcery.com \
--cc=libc-alpha@sourceware.org \
--cc=macro@wdc.com \
--cc=palmer@sifive.com \
--cc=zongbox@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).