21.11.2024 15:57, Alexander Mikhalitsyn пишет:
Hi Stas!
Hmm, it is a bit unusual that SCM_PIDFD message format is different in case when you send it and when you read it.
I mean that when you read it (on the receiver side) you get pidfd file descriptor number, while when you write it (on the sender side) you are only allowed to send one integer and this time it's a pidfd file descriptor flags. I personally have nothing strictly against that but just found this a bit unusual and probably confusing for userspace programmers.
Compare it with SCM_CREDENTIALS, for instance, where we read/write the same structure struct ucred.
SCM_PIDFD_FLAGS can be added instead. But as you currently can't send SCM_PIDFD, and won't be able to receive SCM_PIDFD_FLAGS if it is added, then nothing prevents the reuse of an existing value. So among the possible solutions is to have SCM_PIDFD_FLAGS as an alias to SCM_PIDFD.
Should I do that?
goto error;memcpy(&flags, CMSG_DATA(cmsg), sizeof(flags));err = pidfd_validate_flags(flags);pidfd_validate_flags allows PIDFD_THREAD, but what's the idea behind this if scm->pid is always a thread-group leader? (see maybe_add_creds() function).
Sorry if I misunderstand something just want to ensure that we are on the same page.
The idea is to cover only PIDFD_NONBLOCK for now, and add new flags after. Of course if PIDFD_NONBLOCK+future extensions sounds not very convincing, then I'll have to send those "new flag" patches together with this one. As you can see, the supplied test-case only covers PIDFD_NONBLOCK. You can decide whether or not it is valuable/acceptable on its own. Speaking of PIDFD_THREAD, one can add tid to creds for this to work. I don't need such functionality, so instead I can as well disallow sending this flag until someone else needs it. But it looks potentially useful, so adding tid can also be considered and looks simple enough. What do you think?