[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180829224247.GQ1878@brightrain.aerifal.cx>
Date: Wed, 29 Aug 2018 18:42:47 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Preserve select() behavior on arm64
On Fri, May 25, 2018 at 01:52:40PM -0700, Peter Hurley wrote:
> On Linux, all the select-related syscalls update the timeout value to
> indicate how much elapsed time the syscall consumed, and many programs
> expect this behavior when running on Linux.
>
> Newer archs like arm64 have deprecated the select syscall because the
> pselect syscall is a superset implementation of the select syscall.
>
> A complication of implementing select() with the pselect syscall is
> that the timeouts are specified in different units; select() accepts
> a struct timeval ptr whereas the pselect syscall takes a struct
> timespec ptr. These are trivial convertible; struct timeval is
> specified in seconds + microseconds and struct timespec is in
> seconds + nanoseconds.
>
> For kernel configurations without select syscall available, update the
> caller's struct timeval argument after the pselect syscall.
> ---
> src/select/select.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/src/select/select.c b/src/select/select.c
> index 7b5f6dcf7a53..45d4cb7a3d0a 100644
> --- a/src/select/select.c
> +++ b/src/select/select.c
> @@ -12,15 +12,16 @@ int select(int n, fd_set *restrict rfds, fd_set *restrict wfds, fd_set *restrict
> #else
> syscall_arg_t data[2] = { 0, _NSIG/8 };
> struct timespec ts;
> + int result;
> if (tv) {
> - if (tv->tv_sec < 0 || tv->tv_usec < 0)
> - return __syscall_ret(-EINVAL);
> - time_t extra_secs = tv->tv_usec / 1000000;
> - ts.tv_nsec = tv->tv_usec % 1000000 * 1000;
> - const time_t max_time = (1ULL<<8*sizeof(time_t)-1)-1;
> - ts.tv_sec = extra_secs > max_time - tv->tv_sec ?
> - max_time : tv->tv_sec + extra_secs;
> + ts->tv_sec = tv->tv_sec;
> + ts->tv_nsec = tv->usec * 1000;
> }
> - return syscall_cp(SYS_pselect6, n, rfds, wfds, efds, tv ? &ts : 0, data);
> + result = syscall_cp(SYS_pselect6, n, rfds, wfds, efds, tv ? &ts : 0, data);
> + if (tv) {
> + tv->tv_sec = ts->tv_sec;
> + tv->tv_usec = ts->tv_nsec / 1000;
> + }
> + return result;
> #endif
> }
> --
> 2.14.2
Sorry for not replying to this sooner. I was unsure what to say at
first and then it just slipped my mind to go back to it.
Normally we don't aim to duplicate behavior that is explicitly
documented as non-portable, because programs depending on it should be
fixed (they're already broken on many non-linux systems).
Where it varies by arch because of implementation details like the
above (whether there's a SYS_select or SYS_pselect has to be used)
maybe the principle of consistent behavior across archs has something
to say here.
I'd almost rather do it the opposite (modern/pselect-like) direction,
making a copy of the struct to pass to SYS_select so the caller's copy
doesn't get clobbered, but maybe you like that even less..
Thoughts from anyone else?
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
