[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200903154935.GA3265@brightrain.aerifal.cx>
Date: Thu, 3 Sep 2020 11:49:36 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: [PATCH 07/14] Emulate wait4 using waitid
On Thu, Sep 03, 2020 at 07:23:02AM -0400, Stefan O'Rear wrote:
> riscv32 and future architectures lack wait4.
> ---
> src/internal/wait4_waitid.h | 1 +
> src/linux/__wait4_waitid.c | 52 +++++++++++++++++++++++++++++++++++++
> src/linux/wait4.c | 5 ++++
> src/process/waitpid.c | 6 +++++
> src/stdio/pclose.c | 6 +++++
> src/unistd/faccessat.c | 6 ++++-
> 6 files changed, 75 insertions(+), 1 deletion(-)
> create mode 100644 src/internal/wait4_waitid.h
> create mode 100644 src/linux/__wait4_waitid.c
I really don't like introducing a new src/internal file for this. If
the code needs to be shared it should probably just be in wait4.c,
renamed to __wait4, declared in src/include/sys/wait.h according to
the usual pattern for exposing namespace-safe versions of
namespace-violating functions.
Ideally I'd like to just not need it, but I'm not sure that's
possible:
- faccessat throws away the status and doesn't have any need for the
status or idtype emulation code. It could easily be just different
syscalls in #ifdef/#else paths.
- pclose does need status but not the idtype part. but it's only
listening for process termination not all the other weird stuff. I'm
not sure if it would make sense to construct status inline here. It
might if we had a macro like the glibc "inverse-W" macro (I forget
its name) to construct a wait status from parts; if so this could
later be considered for a public API (previously requested).
- waitpid and wait4 at least need the idtype and status construction.
Unfortunately, wait4 also needs the rusage conversion, which waitpid
doesn't want. This kinda defeats my idea of just exposing __wait4 from
wait4.c.
A side observation to keep in mind is that passing the argument cp
doesn't really help optimize anything; it only worked well in
c2feda4e2e because the function there is inline. Of the 4 callers you
have, faccessat and pclose have hard requirements not to be a
cancellation point and waitpid has a hard requirement to be one. But
if the function weren't used for the former, it could just always be
cancellable -- wait4 probably should have been cancellable all along,
and almost surely is on other implementations, so to use it safely
you'd have to not use, block, or handle cancellation.
So I'm leaning towards sticking with something like what you've done,
but with declaration moved to src/include/sys/wait.h, or possibly
src/internal/syscall.h (since it's essentially emulation of a
syscall). Inline in src/internal/syscall.h might also be an option;
one appealing aspect of that is that it would get rid of the #ifdefs
in source files and allow calling __sys_wait4() unconditionally with
no codegen regression on existing archs. This is analogous to
__sys_open[23].
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.
