Re: [PATCH 06/18] New vCtrlC packet, non-stop mode equivalent of \003

[Yao Qi <qiyaoltc@gmail.com>]

Pedro Alves <palves@redhat.com> writes:

> gdb/
> 2015-10-14  Pedro Alves  <palves@redhat.com>
>
> 	* NEWS (New remote packets): Mention vCtrlC.
>
> gdb/doc/
> 2015-10-14  Pedro Alves  <palves@redhat.com>
>
> 	* gdb.texinfo (Bootstrapping): Add
> 	"interrupting remote targets" anchor.
> 	(Packets): Document vCtrlC.
>
> gdb/gdbserver/
> 2015-10-14  Pedro Alves  <palves@redhat.com>
>
> 	* server.c (handle_v_requests): Handle vCtrlC.

These entries below should be hoisted above to gdb/ChangeLog,

> 	* remote.c (PACKET_vCtrlC): New enum value.
> 	(async_remote_interrupt): Call target_interrupt instead of
> 	target_stop.
> 	(remote_interrupt_as): Remove 'ptid' parameter.
> 	(remote_interrupt_ns): New function.
> 	(remote_stop): Adjust.
> 	(remote_interrupt): If the target is in non-stop mode, try
> 	interrupting with vCtrlC.
> 	(initialize_remote): Install set remote ctrl-c packet.
> ---
>  gdb/NEWS               |  4 ++++
>  gdb/doc/gdb.texinfo    | 34 +++++++++++++++++++++++----
>  gdb/gdbserver/server.c |  7 ++++++
>  gdb/remote.c           | 64 ++++++++++++++++++++++++++++++++++++++++++++------
>  4 files changed, 98 insertions(+), 11 deletions(-)
>

>  
> +/* Non-stop version of target_interrupt.  Uses `vCtrlC' to interrupt
> +   the remote target.  It is undefined which thread of which process
> +   reports the interrupt.  */
> +

We need to document the return value of this function in comments.

> +static int
> +remote_interrupt_ns (void)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +  char *p = rs->buf;
> +  char *endp = rs->buf + get_remote_packet_size ();
> +
> +  xsnprintf (p, endp - p, "vCtrlC");
> +
> +  /* In non-stop, we get an immediate OK reply.  The stop reply will
> +     come in asynchronously by notification.  */
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  switch (packet_ok (rs->buf, &remote_protocol_packets[PACKET_vCtrlC]))
> +    {
> +    case PACKET_OK:
> +      break;
> +    case PACKET_UNKNOWN:
> +      return 0;
> +    case PACKET_ERROR:
> +      error (_("Interrupting target failed: %s"), rs->buf);
> +    }
> +
> +  return 1;
> +}

Patch looks good to me otherwise.

-- 
Yao (齐尧)