Re: replace 'target async' by a maintenance command

[Pedro Alves <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