[PATCH v4 2/3] posix: Add pidfd_fork
Florian Weimer
fweimer@redhat.com
Mon May 22 10:12:16 GMT 2023
* 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
as fork, and store the FD through an int * argument?
The documentaiton should discuss if SIGCHLD is sent for such processes,
and if the PID can be used in waitpid etc. Ideally we'd have test cases
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.
> diff --git a/posix/fork-internal.c b/posix/fork-internal.c
> new file mode 100644
> index 0000000000..be0d3219af
> --- /dev/null
> +++ b/posix/fork-internal.c
> +void
> +__fork_pos (int state, uint64_t lastrun, bool multiple_threads,
> + struct nss_database_data *nss_database_data)
Maybe __fork_post, with a t?
> diff --git a/posix/fork-internal.h b/posix/fork-internal.h
> new file mode 100644
> index 0000000000..3814d851c4
> --- /dev/null
> +++ b/posix/fork-internal.h
> +uint64_t __fork_pre (bool, struct nss_database_data *) attribute_hidden;
> +void __fork_pos (int, uint64_t, bool, struct nss_database_data *)
> + attribute_hidden;
I suggest to put the arguments into a struct, with struct
nss_database_data as a non-pointer member. It makes it easier to
document the meaning of the arguments. And we may have to add more
things in the future.
> diff --git a/sysdeps/unix/sysv/linux/pidfd_fork.c b/sysdeps/unix/sysv/linux/pidfd_fork.c
> new file mode 100644
> index 0000000000..a3ef48ce77
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/pidfd_fork.c
> +static int
> +forkfd (void)
> +{
> + /* The kernel should not change the pidfd for the child, and the returning
> + value on the parent is always larger than 0 (since a new file descriptor
> + is always opened with CLONE_PIDFD is the kernel supports it. */
Missing ).
Thanks,
Florian
More information about the Libc-alpha
mailing list