Re: [PATCH v5 2/5] pidfd: add PIDFD_SELF_* sentinels to refer to own thread/process

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/25/24 11:38 AM, Pedro Falcato wrote:
On Fri, Oct 25, 2024 at 6:41 PM John Hubbard <jhubbard@xxxxxxxxxx> wrote:

On 10/25/24 5:50 AM, Pedro Falcato wrote:
On Fri, Oct 25, 2024 at 10:41 AM Lorenzo Stoakes
<lorenzo.stoakes@xxxxxxxxxx> wrote:
...
+static inline int pidfd_is_self_sentinel(pid_t pid)
+{
+       return pid == PIDFD_SELF_THREAD || pid == PIDFD_SELF_THREAD_GROUP;
+}

Do we want this in the uapi header? Even if this is useful, it might
come with several drawbacks such as breaking scripts that parse kernel
headers (and a quick git grep suggests we do have static inlines in
headers, but in rather obscure ones) and breaking C89:


Let's please not say "C89" anymore, we've moved on! :)

The notes in [1], which is now nearly 2.5 years old, discuss the move to
C11, and specifically how to handle the inline keyword.

That seems to only apply to the kernel internally, uapi headers are

Yes.

included from userspace too (-std=c89 -pedantic doesn't know what a
gnu extension is). And uapi headers _generally_ keep to defining
constants and structs, nothing more.

OK

I don't know what the guidelines for uapi headers are nowadays, but we
generally want to not break userspace.


I think it's quite clear at this point, that we should not hold up new
work, based on concerns about handling the inline keyword, nor about
C89.

Right, but the correct solution is probably to move
pidfd_is_self_sentinel to some other place, since it's not even
supposed to be used by userspace (it's semantically useless to
userspace, and it's only two users are in the kernel, kernel/pid.c and
exit.c).


Yes, if userspace absolutely doesn't need nor want this, then putting
it in a non-uapi header does sound like the right move.


thanks,
--
John Hubbard





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux