From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 12602 invoked from network); 3 Sep 2020 15:49:51 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 3 Sep 2020 15:49:51 -0000 Received: (qmail 8174 invoked by uid 550); 3 Sep 2020 15:49:49 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 8153 invoked from network); 3 Sep 2020 15:49:48 -0000 Date: Thu, 3 Sep 2020 11:49:36 -0400 From: Rich Felker To: musl@lists.openwall.com Message-ID: <20200903154935.GA3265@brightrain.aerifal.cx> References: <20200903112309.102601-1-sorear@fastmail.com> <20200903112309.102601-8-sorear@fastmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200903112309.102601-8-sorear@fastmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [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