RE: [PATCH v2 1/3] gdb: parameterize all_threads_safe for the target and ptid

["Aktemur, Tankut Baris" <tankut.baris.aktemur@intel.com>]

On Monday, April 20, 2026 6:58 PM, Simon Marchi wrote:
> On 4/20/26 9:46 AM, Tankut Baris Aktemur wrote:
> > diff --git a/gdb/infrun.c b/gdb/infrun.c
> > index b295956f8ea..46d6b684ebc 100644
> > --- a/gdb/infrun.c
> > +++ b/gdb/infrun.c
> > @@ -1239,7 +1239,7 @@ static void
> >  follow_exec (ptid_t ptid, const char *exec_file_target)
> >  {
> >    int pid = ptid.pid ();
> > -  ptid_t process_ptid;
> > +  ptid_t process_ptid = ptid_t (pid);
> 
> ptid_t process_ptid (pid);
> 
> >    /* Switch terminal for any messages produced e.g. by
> >       breakpoint_re_set.  */
> > @@ -1285,8 +1285,9 @@ follow_exec (ptid_t ptid, const char *exec_file_target)
> >       them.  Deleting them now rather than at the next user-visible
> >       stop provides a nicer sequence of events for user and MI
> >       notifications.  */
> > -  for (thread_info &th : all_threads_safe ())
> > -    if (th.ptid.pid () == pid && th.ptid != ptid)
> > +  process_stratum_target *target = current_inferior ()->process_target ();
> > +  for (thread_info &th : all_threads_safe (target, process_ptid))
> > +    if (th.ptid != ptid)
> 
> Just noting that this is a behavior change, the previous code iterated
> threads of all targets, while the new code only iterates the threads of
> the current process target.  You could have passed nullptr to preserve
> existing behavior.  But I think this is a good change, it's the same
> spirit as your other patches.

Ack.

> > diff --git a/gdb/remote.c b/gdb/remote.c
> > index 88668b2748e..7c8aed481f2 100644
> > --- a/gdb/remote.c
> > +++ b/gdb/remote.c
> > @@ -4584,11 +4584,8 @@ remote_target::update_thread_list ()
> >        /* CONTEXT now holds the current thread list on the remote
> >  	 target end.  Delete GDB-side threads no longer found on the
> >  	 target.  */
> > -      for (thread_info &tp : all_threads_safe ())
> > +      for (thread_info &tp : all_threads_safe (this))
> >  	{
> > -	  if (tp.inf->process_target () != this)
> > -	    continue;
> > -
> 
> You could now remove the braces for the `for`, to reduce indentation a
> bit.

There is another loop right after this one where we can apply
the same rule.  Shall I remove braces there, too, in another
obvious patch?  Otherwise, we will have inconsistency within
the function.

> >  	  if (!context.contains_thread (tp.ptid))
> >  	    {
> >  	      /* Do not remove the thread if it is the last thread in
> > diff --git a/gdb/thread-iter.h b/gdb/thread-iter.h
> > index 3c15ada1cbb..32bc6475ebe 100644
> > --- a/gdb/thread-iter.h
> > +++ b/gdb/thread-iter.h
> > @@ -165,7 +165,7 @@ using inf_non_exited_threads_iterator
> >  /* Iterate over all threads of all inferiors, safely.  */
> >
> >  using all_threads_safe_iterator
> > -  = basic_safe_iterator<all_threads_iterator>;
> > +  = basic_safe_iterator<all_matching_threads_iterator>;
> 
> I'd rename all_threads_safe_iterator to
> all_matching_threads_safe_iterator, to try to preserve the logic in how
> these are named.
> 
> all_threads_safe_range would also be renamed to
> all_matching_threads_safe_range.
> 
> With those fixed:
> 
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
> 
> Simon

Thank you.
-Baris

Intel Deutschland GmbH
Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany
Tel: +49 89 991 430, www.intel.de
Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell
Chairperson of the Supervisory Board: Nicole Lau
Registered Seat: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928