Re: [patch 4/8] `use_agent' for remote and QAgent

[Pedro Alves <palves@redhat.com>]

On 01/23/2012 01:53 PM, Yao Qi wrote:
> This patch is to implement the to_use_agent in target_ops in remote
> target.  A new packet `QAgent' is added to set the flag in remote stub.
> 
> -- Yao (齐尧)
> 
> 
> 0004-impl-of-to_use_agent-for-remote.patch
> 
> 
> gdb:
> 
> 2012-01-23  Yao Qi  <yao@codesourcery.com>
> 
> 	* remote.c (remote_use_agent): New.
> 	(init_remote_ops): Install `remote_use_agent'.
> 
> gdb/gdbserver:
> 
> 2012-01-23  Yao Qi  <yao@codesourcery.com>
> 
> 	* server.c (handle_general_set): Handle packet 'QAgent'.
> 
> gdb/doc:
> 
> 2012-01-23  Yao Qi  <yao@codesourcery.com>
> 
> 	* gdb.texinfo (General Query Packets): Add packet `QAgent'.
> ---
>  gdb/doc/gdb.texinfo    |   12 ++++++++++++
>  gdb/gdbserver/server.c |   26 ++++++++++++++++++++++++++
>  gdb/remote.c           |   18 ++++++++++++++++++
>  3 files changed, 56 insertions(+), 0 deletions(-)
> 
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 49db189..c4763d4 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -34486,6 +34486,10 @@ Here are the currently defined query and set packets:
>  
>  @table @samp
>  
> +@item QAgent:1
> +@item QAgent:0
> +Turn on or off agent as a helper.
> +
>  @item QAllow:@var{op}:@var{val}@dots{}
>  @cindex @samp{QAllow} packet
>  Specify which operations @value{GDBN} expects to request of the
> @@ -35065,6 +35069,11 @@ These are the currently defined stub features and their properties:
>  @tab @samp{-}
>  @tab No
>  
> +@item @samp{QAgent}
> +@tab No
> +@tab @samp{-}
> +@tab No
> +
>  @item @samp{QAllow}
>  @tab No
>  @tab @samp{-}
> @@ -35198,6 +35207,9 @@ The remote stub accepts and implements the reverse step packet
>  The remote stub understands the @samp{QTDPsrc} packet that supplies
>  the source form of tracepoint definitions.
>  
> +@item QAgent
> +The remote stub understaqnds the @samp{QAgent} packet.
> +
>  @item QAllow
>  The remote stub understands the @samp{QAllow} packet.
>  
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index bebccf5..a5e5bbb 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -424,6 +424,8 @@ write_qxfer_response (char *buf, const void *data, int len, int is_more)
>  			       PBUFSIZ - 2) + 1;
>  }
>  
> +extern int use_agent;

Wasn't this declared in some header?

> +
>  /* Handle all of the extended 'Q' packets.  */
>  
>  static void
> @@ -529,6 +531,30 @@ handle_general_set (char *own_buf)
>        && handle_tracepoint_general_set (own_buf))
>      return;
>  
> +  if (strncmp ("QAgent:", own_buf, strlen ("QAgent:")) == 0)
> +    {
> +      char *mode = own_buf + strlen ("QAgent:");
> +      int req = 0;
> +
> +      if (strcmp (mode, "0") == 0)
> +	req = 0;
> +      else if (strcmp (mode, "1") == 0)
> +	req = 1;
> +      else
> +	{
> +	  /* We don't know what this value is, so complain to GDB.  */
> +	  fprintf (stderr, "Unknown QAgent value requested: %s\n", own_buf);
> +	  write_enn (own_buf);

You can't send the error message back to gdb, with the E.foo form.
WDYT?

> +	  return;
> +	}
> +
> +      /* Update the flag.  */
> +      use_agent = req;
> +      if (remote_debug)
> +	fprintf (stderr, "[%s agent]\n", req ? "Enable" : "Disable");
> +      write_ok (own_buf);
> +    }
> +
>    /* Otherwise we didn't know what packet it was.  Say we didn't
>       understand it.  */
>    own_buf[0] = 0;
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 60d7ecd..50bb90a 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -10581,6 +10581,23 @@ remote_set_trace_notes (char *user, char *notes, char *stop_notes)
>    return 1;
>  }
>  
> +extern int use_agent;

Likewise.

> +
> +static int
> +remote_use_agent (int use)
> +{
> +  struct remote_state *rs = get_remote_state ();
> +
> +  sprintf (rs->buf, "QAgent:%d", use);
> +  putpkt (rs->buf);
> +  getpkt (&rs->buf, &rs->buf_size, 0);
> +
> +  if (strcmp (rs->buf, "OK") == 0)
> +    use_agent = use;
> +
> +  return use_agent;
> +}

So what is supposed to happen if you do "set agent on" and the remote
errors, or doesn't support the packet?  At least errors should be handled.
Please consider what happens to the command's setting in those cases
as well.

> +
>  static void
>  init_remote_ops (void)
>  {
> @@ -10684,6 +10701,7 @@ Specify the serial device it is connected to\n\
>    remote_ops.to_static_tracepoint_markers_by_strid
>      = remote_static_tracepoint_markers_by_strid;
>    remote_ops.to_traceframe_info = remote_traceframe_info;
> +  remote_ops.to_use_agent = remote_use_agent;
>  }
>  
>  /* Set up the extended remote vector by making a copy of the standard
> -- 1.7.0.4


-- 
Pedro Alves