On 20/05/24 05:53, Alejandro Colomar wrote: > Oops, I mistyped the glibc list. Below is included the original email. > > --- > > On Mon, May 20, 2024 at 07:02:39AM GMT, Emanuele Torre wrote: >> Hello. > > Hi Emanuele, > >> pidfd_open(2) only lists sys/syscall.h and unistd.h in its SYNOPSYS: >> >> SYNOPSIS >> #include <sys/syscall.h> /* Definition of SYS_* constants */ >> #include <unistd.h> >> >> int syscall(SYS_pidfd_open, pid_t pid, unsigned int flags); >> >> Note: glibc provides no wrapper for pidfd_open(), necessitating >> the use of syscall(2). >> >> Then it mentions PIDFD_NONBLOCK as one of its flags: >> >> PIDFD_NONBLOCK (since Linux 5.10) >> Return a nonblocking file descriptor. If the process referred >> to by the file descriptor has not yet terminated, then an at‐ >> tempt to wait on the file descriptor using waitid(2) will imme‐ >> diately return the error EAGAIN rather than blocking. >> >> But PIDFD_NONBLOCK is not defined in any of the listed headers. > > Hmmm. Thanks! We need to add its header. > >> I have noticed that PIDFD_NONBLOCK is the same value as O_NONBLOCK, >> so perhaps this flag could be listed as >> >> O_NONBLOCK or PIDFD_NONBLOCK (since Linux 5.10) >> >> like O_NDELAY and O_NONBLOCK in open.2. >> >> This way the user would know that O_NONBLOCK may be used instead of >> PIDFD_NONBLOCK if PIDFD_NONBLOCK is not available. > > No. That's an implementation detail, which shouldn't be abused. > >> I have also noticed that GNU libc (in its linux-api-headers submodule) >> provides a linux/pidfd.h header that just defines PIDFD_NONBLOCK as >> O_NONBLOCK, perhaps another solution would be to list in linux/pidfd.h >> in the synopsis and say it is required to use PIDFD_NONBLOCK. > > Yep, that's the kernel uapi header. I didn't know glibc redistributes > those. > > Anyway, we should indeed include <linux/pidfd.h> for this macro. The glibc provides the pidfd_open, pidfd_getfd, and pidfd_send_signal since 2.36 [1][2][3], and pidfd_getpid since 2.39 [4]. It also provides the pidfd_spawn and pidfd_spawp [5], which are similar to posix_spawn, but return return a pidfd. > >> Then, I also noticed that GNU libc now also provides the sys/pidfd.h >> header with the definition of PIDFD_NONBLOCK, and prototypes for >> pidfd_open, pidfd_send_signal, pidfd_getfd, and also a prototype for >> pidfd_getpid that is an helper function that parses the "Pid:" field >> from /proc/self/fdinfo/FD and currently does not have a man page. > > Hmmm, I've CCed glibc for a question: When you provide a macro like > this one, without providing a syscall wrapper, should we include the > glibc header or the kernel one? Do you provide all kernel uapi macros, > or just select ones? For pidfd function we decided to add the function on sys/pidfd.h which is a distinct header. Maybe we should follow other kernel header integration and include it if existent and only define the required macros if not existent (like we do on mount.h). > > As far as I understand (I have never tried to use it in a program), > > pid_t pid = pidfd_getfd(pidfd); > > Is equivalent to the following command in shell: > > pid=$(grep -Pom1 '^Pid:\t\K.*' proc/self/fdinfo"$pidfd" || echo -1) Yes, and it sets errno depending parsing and 'Pid:' value (you can check on the pidfd_getfd documentation on glibc manual). [1] https://sourceware.org/git/?p=glibc.git;a=commit;h=97f5d19c45799e3abedef771430b5562f1b8764f [2] https://sourceware.org/git/?p=glibc.git;a=commit;h=32dd8c251a431c90451092653f0231a4ad2665e5 [3] https://sourceware.org/git/?p=glibc.git;a=commit;h=56cf9e8eec3bdc0ce44efeda373de9d6b825ea1e [4] https://sourceware.org/git/?p=glibc.git;a=commit;h=e7190fc73dbc8a1f8f94f8ccacd9a190fa5e609c [5] https://sourceware.org/git/?p=glibc.git;a=commit;h=0d6f9f626521678f330f8bfee89e1cdb7e2b1062 > >> >> So probably the best solution is to just make the pidfd_open(2), >> pidfd_send_signal(2), and pidfd_getfd(2) man pages tell users to include >> sys/pidfd.h and call the GNU libc functions instead of including >> sys/syscall.h and unistd.h and calling syscall(2) directly; now that >> sys/pidfd.h exists. > > Ahh, interesting. I'm using glibc 2.38 and still don't have that one. > It seems added in 2.39. We can directly document that in > pidfd_getfd(2). > >> And maybe to also add a pidfd_getpid(3) man page for the new pidfd >> helper function. > > No, usually we document the glibc wrapper in man2, unless there's a big > difference between the kernel syscall and the glibc wrapper. > > Thanks for the detailed report! > > Have a lovely day! > Alex > >> >> >> o/ >> emanuele6 >
