[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180829225639.GR1878@brightrain.aerifal.cx>
Date: Wed, 29 Aug 2018 18:56:39 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH] Preserve select() behavior on arm64
On Wed, Aug 29, 2018 at 06:42:47PM -0400, Rich Felker wrote:
> 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.
I think the patch is also incorrect as written. It removes the code
for handling timeval values which would be out of range if naively
converted to timespec (either<0 or usec>999999, and overflows). I'm
not sure how SYS_select handles updating the remaining time in the
overflow cases, so if you're trying to duplicate it, researching that
is probably called for. It may be right to just normalize it like
you're doing when converting back, but this will effectively report
the wrong amount of elapsed time if tv_sec+tv_usec/1000000 overflows.
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.
