Re: [PATCH v3 5/7] gdb/remote.c: refactor pending fork status functions

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2021-12-03 18:43, Pedro Alves wrote:
> On 2021-12-01 14:44, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> In preparation for a following patch, refactor a few things that I did
>> find a bit awkward, and to make them a bit more reusable.
>>
>>  - Pass an inferior to kill_new_fork_children instead of a pid.  That
>>    allows iterating on only this inferior's threads and avoid further
>>    filtering on the thread's pid.
>>  - Change thread_pending_fork_status to return a non-nullptr value only
>>    if the thread does have a pending fork status.
>>  - Remove is_pending_fork_parent_thread, as one can just use
>>    thread_pending_fork_status and check for nullptr.
>>  - Replace is_pending_fork_parent with is_fork_status, which just
>>    returns if the given targeet_waitkind if a fork or a vfork.  Push
> 
> typo "targeet_waitkind".

Fixed.

>> -/* Return the thread's pending status used to determine whether the
>> -   thread is a fork parent stopped at a fork event.  */
>> +/* Return THREAD's pending status if the it is a pending fork parent, else
>> +   return nullptr.  */
> 
> Some typo in "if the it is".

Fixed.

>> -static const target_waitstatus &
>> +static const target_waitstatus *
>>  thread_pending_fork_status (struct thread_info *thread)
>>  {
> 
> 
>> -/* Kill any new fork children of process PID that haven't been
>> +/* Kill any new fork children of inferior INF that haven't been
>>     processed by follow_fork.  */
>>  
>>  void
>> -remote_target::kill_new_fork_children (int pid)
>> +remote_target::kill_new_fork_children (inferior *inf)
>>  {
>>    remote_state *rs = get_remote_state ();
>>    struct notif_client *notif = &notif_client_stop;
>>  
>>    /* Kill the fork child threads of any threads in process PID
>>       that are stopped at a fork event.  */
> 
> Stale comment, refers to PID.

Fixed.

>> -  for (thread_info *thread : all_non_exited_threads (this))
>> +  for (thread_info *thread : inf->non_exited_threads ())
>>      {
>> -      const target_waitstatus &ws = thread->pending_follow;
>> +      const target_waitstatus *ws = thread_pending_fork_status (thread);
>>  
>> -      if (is_pending_fork_parent (ws, pid, thread->ptid))
>> -	{
>> -	  int child_pid = ws.child_ptid ().pid ();
>> -	  int res;
>> +      if (ws == nullptr)
>> +	continue;
>>  
>> -	  res = remote_vkill (child_pid);
>> -	  if (res != 0)
>> -	    error (_("Can't kill fork child process %d"), child_pid);
>> -	}
>> +      int child_pid = ws->child_ptid ().pid ();
>> +      int res = remote_vkill (child_pid);
>> +
>> +      if (res != 0)
>> +	error (_("Can't kill fork child process %d"), child_pid);
>>      }
>>  
>>    /* Check for any pending fork events (not reported or processed yet)
>>       in process PID and kill those fork child threads as well.  */
> 
> Ditto.

Fixed.

> Otherwise LGTM.

Thanks,

Simon