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

Yao Qi
2015-10-26T11:45:06Z

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 (齐尧)