Re: [PATCH]Add option "-at" to "maint agent" and "maint agent-eval"

[Eli Zaretskii <eliz at gnu dot org>]

> From: Hui Zhu <teawater@gmail.com>
> Date: Sun, 24 Jun 2012 14:59:08 +0800
> Cc: Stan Shebs <stan_shebs@mentor.com>, Yao Qi <Yao_Qi@mentor.com>, 
> 	Eli Zaretskii <eliz@gnu.org>
> According the comments form Stan in
> http://sourceware.org/ml/gdb/2012-06/msg00060.html
> I post these patches for add option "-at"  to "maint agent" and "maint
> agent-eval".

Thanks.

> 
> Thanks,
> Hui
> 
> 2012-06-24  Hui Zhu  <hui_zhu@mentor.com>
> 
> 	* breakpoint.c (check_for_argument): Move to file cli/cli-utils.c.
> 	* cli/cli-utils.c (check_for_argument): New function.
> 	* cli/cli-utils.h (check_for_argument): Ditto.
> 
> 2012-06-24  Hui Zhu  <hui_zhu@mentor.com>
> 
> 	* ax-gdb.c (cli/cli-utils.h): New include.
> 	(linespec.h): Ditto.
> 	(agent_eval_command_one): New function.
> 	(agent_command_1): Ditto.
> 	(agent_command): Call function agent_command_1.
> 	(agent_eval_command): Ditto.
> 	(_initialize_ax_gdb): Change help for "maint agent"
> 	and "maint agent-eval".
> 
> 2012-06-24  Hui Zhu  <hui_zhu@mentor.com>
> 
> 	* gdb.texinfo (Maintenance Commands): Change help for "maint agent"
> 	and "maint agent-eval".
> 
> [2:text/plain Hide Save:check_for_argument_move_cli_utils.txt (2kB)]
> 
> ---
>  breakpoint.c    |   17 -----------------
>  cli/cli-utils.c |   14 ++++++++++++++
>  cli/cli-utils.h |    6 ++++++
>  3 files changed, 20 insertions(+), 17 deletions(-)
> 
> --- a/breakpoint.c
> +++ b/breakpoint.c
> @@ -10862,23 +10862,6 @@ watch_command_wrapper (char *arg, int fr
>    watch_command_1 (arg, hw_write, from_tty, 0, internal);
>  }
>  
> -/* A helper function that looks for an argument at the start of a
> -   string.  The argument must also either be at the end of the string,
> -   or be followed by whitespace.  Returns 1 if it finds the argument,
> -   0 otherwise.  If the argument is found, it updates *STR.  */
> -
> -static int
> -check_for_argument (char **str, char *arg, int arg_len)
> -{
> -  if (strncmp (*str, arg, arg_len) == 0
> -      && ((*str)[arg_len] == '\0' || isspace ((*str)[arg_len])))
> -    {
> -      *str += arg_len;
> -      return 1;
> -    }
> -  return 0;
> -}
> -
>  /* A helper function that looks for the "-location" argument and then
>     calls watch_command_1.  */
>  
> --- a/cli/cli-utils.c
> +++ b/cli/cli-utils.c
> @@ -286,3 +286,17 @@ extract_arg (char **arg)
>  
>    return copy;
>  }
> +
> +/* See documentation in cli-utils.h.  */
> +
> +int
> +check_for_argument (char **str, char *arg, int arg_len)
> +{
> +  if (strncmp (*str, arg, arg_len) == 0
> +      && ((*str)[arg_len] == '\0' || isspace ((*str)[arg_len])))
> +    {
> +      *str += arg_len;
> +      return 1;
> +    }
> +  return 0;
> +}
> --- a/cli/cli-utils.h
> +++ b/cli/cli-utils.h
> @@ -114,4 +114,10 @@ extern char *remove_trailing_whitespace
>  
>  extern char *extract_arg (char **arg);
>  
> +/* A helper function that looks for an argument at the start of a
> +   string.  The argument must also either be at the end of the string,
> +   or be followed by whitespace.  Returns 1 if it finds the argument,
> +   0 otherwise.  If the argument is found, it updates *STR.  */
> +extern int check_for_argument (char **str, char *arg, int arg_len);
> +
>  #endif /* CLI_UTILS_H */
> 
> [3:text/plain Hide Save:agent-at.txt (6kB)]
> 
> ---
>  ax-gdb.c |  117 ++++++++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 72 insertions(+), 45 deletions(-)
> 
> --- a/ax-gdb.c
> +++ b/ax-gdb.c
> @@ -41,6 +41,8 @@
>  #include "tracepoint.h"
>  #include "cp-support.h"
>  #include "arch-utils.h"
> +#include "cli/cli-utils.h"
> +#include "linespec.h"
>  
>  #include "valprint.h"
>  #include "c-lang.h"
> @@ -2504,40 +2506,32 @@ gen_trace_for_return_address (CORE_ADDR
>  }
>  
>  static void
> -agent_command (char *exp, int from_tty)
> +agent_eval_command_one (char *exp, int eval, CORE_ADDR pc)
>  {
>    struct cleanup *old_chain = 0;
>    struct expression *expr;
>    struct agent_expr *agent;
> -  struct frame_info *fi = get_current_frame ();	/* need current scope */
> -
> -  /* We don't deal with overlay debugging at the moment.  We need to
> -     think more carefully about this.  If you copy this code into
> -     another command, change the error message; the user shouldn't
> -     have to know anything about agent expressions.  */
> -  if (overlay_debugging)
> -    error (_("GDB can't do agent expression translation with overlays."));
> -
> -  if (exp == 0)
> -    error_no_arg (_("expression to translate"));
>  
> -  trace_string_kludge = 0;
> -  if (*exp == '/')
> -    exp = decode_agent_options (exp);
> +  if (!eval)
> +    {
> +      trace_string_kludge = 0;
> +      if (*exp == '/')
> +        exp = decode_agent_options (exp);
> +    }
>  
> -  /* Recognize the return address collection directive specially.  Note
> -     that it is not really an expression of any sort.  */
> -  if (strcmp (exp, "$_ret") == 0)
> +  if (!eval && strcmp (exp, "$_ret") == 0)
>      {
> -      agent = gen_trace_for_return_address (get_frame_pc (fi),
> -					    get_current_arch ());
> +      agent = gen_trace_for_return_address (pc, get_current_arch ());
>        old_chain = make_cleanup_free_agent_expr (agent);
>      }
>    else
>      {
> -      expr = parse_expression (exp);
> +      expr = parse_exp_1 (&exp, block_for_pc (pc), 0);
>        old_chain = make_cleanup (free_current_contents, &expr);
> -      agent = gen_trace_for_expr (get_frame_pc (fi), expr);
> +      if (eval)
> +	agent = gen_eval_for_expr (pc, expr);
> +      else
> +	agent = gen_trace_for_expr (pc, expr);
>        make_cleanup_free_agent_expr (agent);
>      }
>  
> @@ -2551,18 +2545,9 @@ agent_command (char *exp, int from_tty)
>    dont_repeat ();
>  }
>  
> -/* Parse the given expression, compile it into an agent expression
> -   that does direct evaluation, and display the resulting
> -   expression.  */
> -
>  static void
> -agent_eval_command (char *exp, int from_tty)
> +agent_command_1 (char *exp, int eval)
>  {
> -  struct cleanup *old_chain = 0;
> -  struct expression *expr;
> -  struct agent_expr *agent;
> -  struct frame_info *fi = get_current_frame ();	/* need current scope */
> -
>    /* We don't deal with overlay debugging at the moment.  We need to
>       think more carefully about this.  If you copy this code into
>       another command, change the error message; the user shouldn't
> @@ -2573,19 +2558,55 @@ agent_eval_command (char *exp, int from_
>    if (exp == 0)
>      error_no_arg (_("expression to translate"));
>  
> -  expr = parse_expression (exp);
> -  old_chain = make_cleanup (free_current_contents, &expr);
> -  agent = gen_eval_for_expr (get_frame_pc (fi), expr);
> -  make_cleanup_free_agent_expr (agent);
> -  ax_reqs (agent);
> -  ax_print (gdb_stdout, agent);
> +  if (check_for_argument (&exp, "-at", sizeof ("-at") - 1))
> +    {
> +      struct linespec_result canonical;
> +      int ix;
> +      struct linespec_sals *iter;
> +      struct cleanup *old_chain;
>  
> -  /* It would be nice to call ax_reqs here to gather some general info
> -     about the expression, and then print out the result.  */
> +      exp = skip_spaces (exp);
> +      init_linespec_result (&canonical);
> +      decode_line_full (&exp, DECODE_LINE_FUNFIRSTLINE,
> +			(struct symtab *) NULL, 0, &canonical,
> +			NULL, NULL);
> +      old_chain = make_cleanup_destroy_linespec_result (&canonical);
> +      exp = skip_spaces (exp);
> +      if (exp[0] == ',')
> +        {
> +	  exp++;
> +	  exp = skip_spaces (exp);
> +	}
> +      for (ix = 0; VEC_iterate (linespec_sals, canonical.sals, ix, iter); ++ix)
> +        {
> +	  int i;
> +
> +	  for (i = 0; i < iter->sals.nelts; i++)
> +	    agent_eval_command_one (exp, eval, iter->sals.sals[i].pc);
> +        }
> +      do_cleanups (old_chain);
> +    }
> +  else
> +    agent_eval_command_one (exp, eval, get_frame_pc (get_current_frame ()));
>  
> -  do_cleanups (old_chain);
>    dont_repeat ();
>  }
> +
> +static void
> +agent_command (char *exp, int from_tty)
> +{
> +  agent_command_1 (exp, 0);
> +}
> +
> +/* Parse the given expression, compile it into an agent expression
> +   that does direct evaluation, and display the resulting
> +   expression.  */
> +
> +static void
> +agent_eval_command (char *exp, int from_tty)
> +{
> +  agent_command_1 (exp, 1);
> +}
>  
>  
>  /* Initialization code.  */
> @@ -2595,12 +2616,18 @@ void
>  _initialize_ax_gdb (void)
>  {
>    add_cmd ("agent", class_maintenance, agent_command,
> -	   _("Translate an expression into "
> -	     "remote agent bytecode for tracing."),
> +	   _("\
> +Translate an expression into remote agent bytecode for tracing.\n\
> +Usage: maint agent [-at location,] EXPRESSION\n\
> +If -at is given, generate remote agent bytecode for this loation.\n\
                                                            ^^^^^^^
A typo.

> +If -at is given, generate remote agent bytecode for this loation.\n\

And here.

> +@item maint agent @r{[}-at location@r{,}@r{]} @var{expression}
> +@itemx maint agent-eval @r{[}-at location@r{,}@r{]} @var{expression}

"location" should be in @var, as it is a parameter, like
"expression".

> +If @code{-at} is given, generate remote agent bytecode for this loation.

Same typo as before, and please use "... for @var{location}", without
"this".

Btw, what does it mean to generate bytecode "for LOCATION"? how does
the location come into play when bytecode is generated?

> +If not, generate remote agent bytecode for current frame pc address.

Don't we use "PC", in caps, usually?

OK with those changes.