Re: [PATCH v4 2/3] posix: Add pidfd_fork

[Christian Brauner <brauner@kernel.org>]

On Mon, May 22, 2023 at 12:12:16PM +0200, Florian Weimer via Libc-alpha wrote:
> * Adhemerval Zanella:
> 
> > +@deftypefun int pidfd_fork (unsigned int @var{flags})
> > +@standards{GNU, sys/pidfd.h}
> > +@safety{@prelim{}@mtsafe{}@assafe{}@acsafe{}}
> > +The @code{fork} function is similar to @code{fork} but return a file
> > +descriptor instead of process ID.
> > +
> > +If the operation is sucessful, there are both parent and child processes
> > +and both see @code{pidfd_fork} return, but with different values: it return
> > +a value of @code{0} in the child process and returns a file descriptor to
> > +child's process int the parent process.  Although the value of @code{0} clashes
> > +with @code{STDIN_FILENO}, it also assumed that stardand input is always
> > +available so a conformant application will never see a the @code{0} stream
> > +being returned to parent as a valid file descriptor.
> 
> I think this is a bit ugly.  Maybe pidfd_fork should return the PID/zero

I agree. We went through some pain to ensure that 0 can be used as a
valid fd value. Imho, it'd be better to use a return argument. It would
also align with how CLONE_PIDFD with legacy clone() works.

> as fork, and store the FD through an int * argument?
> 
> The documentaiton should discuss if SIGCHLD is sent for such processes,

Depends on whether you set exit_signal. I would expect pidfd_fork() to
very closely mirror the semantics of fork(). So I would expect SIGCHLD
to be delivered. But that's not how I understand this patchset just from
glancing at it.

> and if the PID can be used in waitpid etc.  Ideally we'd have test cases

The pid can be used with waitpid(). I don't understand why it wouldn't.

> for this behavior, too. 8-)

> 
> I think it would be valuable to support creating subprocesses without
> ever sending SIGCHLD on their termination.  Not sure if this should be
> the default behavior, or something enabled with a flag.

Best behind a flag. If you name it pidfd_fork() then it might be best if
you default to delivering SIGCHLD and provide an opt-out through a
flag.