Re: [PATCH v2 05/29] Support clone events in the remote protocol

[Simon Marchi <simark@simark.ca>]

Just some nits:

> @@ -5897,16 +5901,35 @@ is_fork_status (target_waitkind kind)
>  	  || kind == TARGET_WAITKIND_VFORKED);
>  }
>  
> -/* Return THREAD's pending status if it is a pending fork parent, else
> -   return nullptr.  */
> +/* Determine if WS represents an event with a new child - a fork,
> +   vfork, or clone.  */
> +
> +static bool
> +is_new_child_status (target_waitkind kind)
> +{
> +  return (kind == TARGET_WAITKIND_FORKED
> +	  || kind == TARGET_WAITKIND_VFORKED
> +	  || kind == TARGET_WAITKIND_THREAD_CLONED);
> +}

This could maybe be a shared util function in waitstatus.h, linux-nat
has something really similar.

> +
> +/* Return a reference to the field that records the THREAD's pending
> +   child status, if there's one.  */
> +
> +static const target_waitstatus &
> +thread_pending_status (struct thread_info *thread)
> +{
> +  return (thread->has_pending_waitstatus ()
> +	  ? thread->pending_waitstatus ()
> +	  : thread->pending_follow);
> +}

The "if there's one" part of the comment doesn't really work.  We return
a reference, so we will always return something.

> @@ -7303,13 +7346,14 @@ remote_target::remove_new_fork_children (threads_listing_context *context)
>        context->remove_thread (ws->child_ptid ());
>      }
>  
> -  /* Check for any pending fork events (not reported or processed yet)
> -     in process PID and remove those fork child threads from the
> -     CONTEXT list as well.  */
> +  /* Check for any pending (v)fork/clone events (not reported or
> +     processed yet) in process PID and remove those child threads from
> +     the CONTEXT list as well.  */
>    remote_notif_get_pending_events (notif);
>    for (auto &event : get_remote_state ()->stop_reply_queue)
>      if (event->ws.kind () == TARGET_WAITKIND_FORKED
> -	|| event->ws.kind () == TARGET_WAITKIND_VFORKED)
> +	|| event->ws.kind () == TARGET_WAITKIND_VFORKED
> +	|| event->ws.kind () == TARGET_WAITKIND_THREAD_CLONED)

This could use is_new_child_status.

The function could maybe be renamed to just "remove_new_children".

Simon