Re: rust: wrong SAFETY comments in group_leader() and pid() + questions

[Gary Guo <gary@garyguo.net>]

On Fri, 5 Dec 2025 18:17:09 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Fri, Dec 05, 2025 at 06:21:46PM +0100, Oleg Nesterov wrote:
> > On 12/05, Alice Ryhl wrote:  
> > >
> > > pub fn group_leader(&self) -> &Task {
> > >     // SAFETY: The lifetime of the returned task reference is tied to
> > >     // the lifetime of `self`, and given that a task has a reference to
> > >     // its group leader, we know it must be valid for the lifetime of
> > >     // the returned task reference.
> > >     unsafe { &*bindings::task_group_leader(self.as_ptr()).cast::<Task>() }
> > > }  
> > 
> > Thanks again Alice, but the comment still looks misleading to me...
> > OK, quite possibly this is because I don't understand what does the
> > "lifetime of the returned task reference" actually mean in the rust code.
> > Does it mean "lifetime of task_struct" of "lifetime of the process/thread" ?  
> 
> To start with, it's likely that this comment is not the right choice
> for this function, given our discussion. Most likely group_leader()
> needs to be moved to `impl CurrentTask {}` and the safety comment needs
> to explain why being the current task ensures that the returned &Task
> lives for long enough. I just took the safety comment from the code we
> have today.

This indeed sounds like the right approach to take.

If `Task::pid` or `Task::group_leader` just gives the pid or group
leader at the time of invocation and doesn't have any stability
guarantee, then the user of the functions will likely be misusing these
functions.

It's better to just have them on `CurrentTask` for now. When an user
arises that need to retrieve them for another task, then we can come
back and think about a proper solution taking the scenario into account.

Best,
Gary