This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: replace 'target async' by a maintenance command
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Sat, 17 May 2008 20:15:15 +0100
- Subject: Re: replace 'target async' by a maintenance command
- References: <200805171558.21392.pedro@codesourcery.com>
A Saturday 17 May 2008 15:58:21, Pedro Alves escreveu:
> As preparation for non-stop and multi-process support in the remote
> targets, I'd like to get rid of the current async duplication in remote.c.
>
> The remote_wait/remote_async_wait merge is a bit hard to see in the
> attached patch. Below is a diff of both functions I used
> to make sure nothing was left behind.
>
> To get async behaviour, enable remote async support with 'maint set
> remote-async', and use 'target remote' to connect as normal. Target
> async and target extended-async are gone.
>
> Here's remote_wait/remote_async_wait:
>
> --- remote_sync.txt 2008-05-17 15:45:13.000000000 +0100
> +++ remote_async.txt 2008-05-17 15:45:33.000000000 +0100
> @@ -1,5 +1,5 @@
> static ptid_t
> -remote_wait (ptid_t ptid, struct target_waitstatus *status)
> +remote_async_wait (ptid_t ptid, struct target_waitstatus *status)
> {
> struct remote_state *rs = get_remote_state ();
> struct remote_arch_state *rsa = get_remote_arch_state ();
> @@ -10,6 +10,8 @@ remote_wait (ptid_t ptid, struct target_
> status->kind = TARGET_WAITKIND_EXITED;
> status->value.integer = 0;
>
> + remote_stopped_by_watchpoint_p = 0;
> +
> while (1)
> {
> char *buf, *p;
> @@ -19,22 +21,28 @@ remote_wait (ptid_t ptid, struct target_
> rs->cached_wait_status = 0;
> else
> {
> - ofunc = signal (SIGINT, remote_interrupt);
> - /* If the user hit C-c before this packet, or between packets,
> - pretend that it was hit right here. */
> - if (quit_flag)
> + if (!target_is_async_p ())
> {
> - quit_flag = 0;
> - remote_interrupt (SIGINT);
> + ofunc = signal (SIGINT, remote_interrupt);
> + /* If the user hit C-c before this packet, or between packets,
> + pretend that it was hit right here. */
> + if (quit_flag)
> + {
> + quit_flag = 0;
> + remote_interrupt (SIGINT);
> + }
> }
> - getpkt (&rs->buf, &rs->buf_size, 1);
> - signal (SIGINT, ofunc);
> + /* FIXME: cagney/1999-09-27: If we're in async mode we should
> + _never_ wait for ever -> test on target_is_async_p().
> + However, before we do that we need to ensure that the caller
> + knows how to take the target into/out of async mode. */
> + getpkt (&rs->buf, &rs->buf_size, wait_forever_enabled_p);
> + if (!target_is_async_p ())
> + signal (SIGINT, ofunc);
> }
>
> buf = rs->buf;
>
> - remote_stopped_by_watchpoint_p = 0;
> -
> switch (buf[0])
> {
> case 'E': /* Error of some sort. */
> @@ -64,18 +72,18 @@ remote_wait (ptid_t ptid, struct target_
> char *p1;
> char *p_temp;
> int fieldsize;
> - LONGEST pnum = 0;
> + long pnum = 0;
>
> - /* If the packet contains a register number save it in
> - pnum and set p1 to point to the character following
> - it. Otherwise p1 points to p. */
> + /* If the packet contains a register number, save it
> + in pnum and set p1 to point to the character
> + following it. Otherwise p1 points to p. */
>
> - /* If this packet is an awatch packet, don't parse the
> - 'a' as a register number. */
> + /* If this packet is an awatch packet, don't parse the 'a'
> + as a register number. */
>
> if (strncmp (p, "awatch", strlen("awatch")) != 0)
> {
> - /* Read the ``P'' register number. */
> + /* Read the register number. */
> pnum = strtol (p, &p_temp, 16);
> p1 = p_temp;
> }
> @@ -121,20 +129,20 @@ Packet: '%s'\n"),
> p = p_temp;
> }
> }
> +
> else
> {
> struct packet_reg *reg = packet_reg_from_pnum (rsa, pnum);
> p = p1;
> -
> if (*p++ != ':')
> error (_("Malformed packet(b) (missing colon): %s\n\
> Packet: '%s'\n"),
> p, buf);
>
> if (reg == NULL)
> - error (_("Remote sent bad register number %s: %s\n\
> + error (_("Remote sent bad register number %ld: %s\n\
> Packet: '%s'\n"),
> - phex_nz (pnum, 0), p, buf);
> + pnum, p, buf);
>
> fieldsize = hex2bin (p, regs,
> register_size (current_gdbarch,
> @@ -184,7 +192,10 @@ Packet: '%s'\n"),
> goto got_status;
> case 'O': /* Console output. */
> remote_console_output (buf + 1);
> - continue;
> + /* Return immediately to the event loop. The event loop will
> + still be waiting on the inferior afterwards. */
> + status->kind = TARGET_WAITKIND_IGNORE;
> + goto got_status;
> case '\0':
> if (last_sent_signal != TARGET_SIGNAL_0)
> {
>
And I here's a diff of the old remote_wait and the new remote_wait
with async merged in, for easy of reviewability.
*** remote_sync.txt 2008-05-17 15:45:13.000000000 +0100
--- remote_new.txt 2008-05-17 20:08:25.000000000 +0100
*************** remote_wait (ptid_t ptid, struct target_
*** 19,34 ****
rs->cached_wait_status = 0;
else
{
! ofunc = signal (SIGINT, remote_interrupt);
! /* If the user hit C-c before this packet, or between packets,
! pretend that it was hit right here. */
! if (quit_flag)
{
! quit_flag = 0;
! remote_interrupt (SIGINT);
}
! getpkt (&rs->buf, &rs->buf_size, 1);
! signal (SIGINT, ofunc);
}
buf = rs->buf;
--- 19,42 ----
rs->cached_wait_status = 0;
else
{
! if (!target_is_async_p ())
{
! ofunc = signal (SIGINT, remote_interrupt);
! /* If the user hit C-c before this packet, or between packets,
! pretend that it was hit right here. */
! if (quit_flag)
! {
! quit_flag = 0;
! remote_interrupt (SIGINT);
! }
}
! /* FIXME: cagney/1999-09-27: If we're in async mode we should
! _never_ wait for ever -> test on target_is_async_p().
! However, before we do that we need to ensure that the caller
! knows how to take the target into/out of async mode. */
! getpkt (&rs->buf, &rs->buf_size, wait_forever_enabled_p);
! if (!target_is_async_p ())
! signal (SIGINT, ofunc);
}
buf = rs->buf;
*************** remote_wait (ptid_t ptid, struct target_
*** 66,74 ****
int fieldsize;
LONGEST pnum = 0;
! /* If the packet contains a register number save it in
! pnum and set p1 to point to the character following
! it. Otherwise p1 points to p. */
/* If this packet is an awatch packet, don't parse the
'a' as a register number. */
--- 74,82 ----
int fieldsize;
LONGEST pnum = 0;
! /* If the packet contains a register number, save it
! in pnum and set p1 to point to the character
! following it. Otherwise p1 points to p. */
/* If this packet is an awatch packet, don't parse the
'a' as a register number. */
*************** Packet: '%s'\n"),
*** 184,190 ****
goto got_status;
case 'O': /* Console output. */
remote_console_output (buf + 1);
! continue;
case '\0':
if (last_sent_signal != TARGET_SIGNAL_0)
{
--- 192,206 ----
goto got_status;
case 'O': /* Console output. */
remote_console_output (buf + 1);
! if (target_can_async_p ())
! {
! /* Return immediately to the event loop. The event loop
! will still be waiting on the inferior afterwards. */
! status->kind = TARGET_WAITKIND_IGNORE;
! goto got_status;
! }
! else
! continue;
case '\0':
if (last_sent_signal != TARGET_SIGNAL_0)
{
> Tested on with a local gdbserver on x86-64-unknown-linux-gnu, in sync
> mode, with and without this patch; and async mode also with and without
> this patch. No regressions in both modes, compared to unpatched.
>
> (applies on top of the kill kill_kludge patch I sent a bit earlier
> today.)
>
> OK?
--
Pedro Alves