[RFC 0/5] Implenent 'frame apply COMMAND', enhance 'thread apply COMMAND'

[RFC 0/5] Implenent 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Philippe Waroquiers]

This patch series :
 * implements a new command
     'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
 * enhance 'thread apply COMMAND' by adding a -FLAGS argument
 * adds some shortcuts commands
 * documents the above in gdb.texinfo and NEWS.

Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
or to all frames.
The optional -FLAGS... argument allows to control what output to produce
and how to handle errors raised when applying COMMAND to a frame.

Some examples usages for this new command:
   frame apply all info frame
      Produce info frame for all frames
   frame apply all p $sp
      For each frame, print the location, followed by the frame sp
   frame apply all -qq p $sp
      Same as before, but -qq flags (q = quiet) indicate to only print
      the frames sp.
   frame apply all -vv p $sp
      Same as before, but -vv flags (v = verbose) indicate to print
      location and source line for each frame.
   frame apply all p some_local_var_somewhere
      Print some_local_var_somewhere in all frames. 'frame apply'
      will abort as soon as the print command fails.
   frame apply all -c p some_local_var_somewhere
      Same as before, but -c flag (c = continue) means to
      print the error and continue applying command in case the
      print command fails.
   frame apply all -s p some_local_var_somewhere
      Same as before, but -s flag (s = silent) means to
      be silent for frames where the print command fails.
      In other words, this allows to 'search' the frame in which
      some_local_var_somewhere can be printed.

'thread apply' command has been enhanced to also accepts a -FLAGS...
argument.

Some examples usages for this new argument:
   thread apply all -s frame apply all -s p some_local_var_somewhere
      Prints the thread id, frame location and some_local_var_somewhere
      value in frames of threads that have such local var.

To make the life of the user easier, the most typical use cases
have shortcuts :
   faas  : shortcut for 'frame apply all -s'
   taas  : shortcut for 'thread apply all -s'
   tfaas : shortcut for 'thread apply all -s frame apply all -s"

An example usage :
   tfaas p some_local_var_somewhere
     same as the longer:
        'thread apply all -s frame apply all -s p some_local_var_somewhere'

The patch serie provides the implementation and all user visible documentation.

What is still missing is:
   * Missing regression tests.
     Not yet done as this is painful, and so is better done once there is
     an agreement on the behaviour.

   * Missing ChangeLog.
     Not yet done as this is painful, and is IMO close to or completely useless.
     So, I will do it for the (first) RFA, once the flow of RFC comments
     stops :).

[RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND

[Philippe Waroquiers]

Also implement the command 'faas COMMAND', a shortcut for
'frame apply all -s COMMAND'
---
 gdb/stack.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 214 insertions(+), 25 deletions(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index ecf1ee8379..61e86ab18b 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -777,7 +777,7 @@ do_gdb_disassembly (struct gdbarch *gdbarch,
    
    SRC_LINE: Print only source line.
    LOCATION: Print only location.
-   LOC_AND_SRC: Print location and source line.
+   SRC_AND_LOC: Print location and source line.
 
    Used in "where" output, and to emit breakpoint or step
    messages.  */
@@ -1687,6 +1687,39 @@ info_frame_command (const char *addr_exp, int from_tty)
   }
 }
 
+/* COUNT specifies the nr of outermost frames we are interested in.
+   trailing_outermost_frame returns the starting frame
+   needed to handle COUNT outermost frames.  */
+
+static struct frame_info*
+trailing_outermost_frame (int count)
+{
+  struct frame_info *current;
+  struct frame_info *trailing;
+
+  trailing = get_current_frame ();
+
+  gdb_assert (count > 0);
+
+  current = trailing;
+  while (current && count--)
+    {
+      QUIT;
+      current = get_prev_frame (current);
+    }
+
+  /* Will stop when CURRENT reaches the top of the stack.
+     TRAILING will be COUNT below it.  */
+  while (current)
+    {
+      QUIT;
+      trailing = get_prev_frame (trailing);
+      current = get_prev_frame (current);
+    }
+
+  return trailing;
+}
+
 /* Print briefly all stack frames or just the innermost COUNT_EXP
    frames.  */
 
@@ -1696,7 +1729,6 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
 {
   struct frame_info *fi;
   int count;
-  int i;
   int py_start = 0, py_end = 0;
   enum ext_lang_bt_status result = EXT_LANG_BT_ERROR;
 
@@ -1756,30 +1788,13 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
 
       if (count_exp != NULL && count < 0)
 	{
-	  struct frame_info *current;
-
-	  count = -count;
-
-	  current = trailing;
-	  while (current && count--)
-	    {
-	      QUIT;
-	      current = get_prev_frame (current);
-	    }
-
-	  /* Will stop when CURRENT reaches the top of the stack.
-	     TRAILING will be COUNT below it.  */
-	  while (current)
-	    {
-	      QUIT;
-	      trailing = get_prev_frame (trailing);
-	      current = get_prev_frame (current);
-	    }
-
+	  trailing = trailing_outermost_frame (-count);
 	  count = -1;
 	}
+      else
+	trailing = get_current_frame ();
 
-      for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi))
+      for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
 	{
 	  QUIT;
 
@@ -2506,9 +2521,148 @@ func_command (const char *arg, int from_tty)
     select_and_print_frame (frame);
 }
 
+/* Apply a GDB command to a all stack frames, or innermost COUNT frames.
+   With a negative COUNT, apply command on outermost -COUNT frames.
+
+   frame apply 3 info frame     Apply 'info frame' to frames 0, 1, 2
+   frame apply -3 info frame    Apply 'info frame' to outermost 3 frames.
+   frame apply all x/i $pc      Apply 'x/i $pc' cmd to all frames.
+   frame apply all -s p local_var_no_idea_in_which_frame
+                If a frame has a local variable called
+                local_var_no_idea_in_which_frame, print frame
+                and value of local_var_no_idea_in_which_frame.
+   frame apply all -sqq p local_var_no_idea_in_which_frame
+                Same as before, but only print the variable value.  */
+
+/* Apply a GDB command to COUNT stack frames, starting at trailing.
+   COUNT -1 means all frames starting at trailing.  WHICH_COMMAND is used
+   for error messages.  */
+static void
+frame_apply_command_count (const char* which_command,
+			   const char *cmd, int from_tty,
+			   struct frame_info *trailing, int count)
+{
+  int print_what_v = 2; /* Corresponding to LOCATION.  */
+  enum print_what print_what[5] =
+    {
+      LOC_AND_ADDRESS, /* Should never be used, this is verbosity 0.  */
+      SRC_LINE,
+      LOCATION,
+      LOC_AND_ADDRESS,
+      SRC_AND_LOC
+    };
+  bool cont;
+  bool silent;
+  struct frame_info *fi;
+
+  if (cmd != NULL)
+    check_for_flags_vqcs (which_command, &cmd,
+			  &print_what_v, 4,
+			  &cont, &silent);
+
+  if (cmd == NULL || *cmd == '\000')
+    error (_("Please specify a command to apply on the selected frames"));
+
+  for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
+    {
+      struct frame_id frame_id = get_frame_id (fi);
+
+      QUIT;
+
+      select_frame (fi);
+      TRY
+	{
+	  std::string cmd_result = execute_command_to_string (cmd, from_tty);
+	  if (!silent || cmd_result.length() > 0)
+	    {
+	      if (print_what_v > 0)
+		print_stack_frame (fi, 1, print_what[print_what_v], 0);
+	      printf_filtered ("%s", cmd_result.c_str ());
+	    }
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  if (!silent)
+	    {
+	      if (print_what_v > 0)
+		print_stack_frame (fi, 1, print_what [print_what_v], 0);
+	      if (cont)
+		printf_filtered ("%s\n", ex.message);
+	      else
+		throw_exception (ex);
+	    }
+	}
+      END_CATCH;
+
+      /* execute_command_to_string might invalidate FI.  */
+      fi = frame_find_by_id (frame_id);
+      if (fi == NULL)
+	{
+	  trailing = NULL;
+	  warning (_("Unable to restore previously selected frame."));
+	  break;
+	}
+    }
+}
+
+static void
+frame_apply_all_command (const char *cmd, int from_tty)
+{
+  struct frame_info *fi;
+  struct frame_info *trailing = NULL;
+  int count;
+  int i;
+
+  if (!target_has_stack)
+    error (_("No stack."));
+
+  frame_apply_command_count ("frame apply all", cmd, from_tty,
+			     get_current_frame(), INT_MAX);
+}
+
+/* Implementation of the "frame apply" command.  */
+
+static void
+frame_apply_command (const char* cmd, int from_tty)
+{
+  int count;
+  struct frame_info *trailing;
+
+  if (!target_has_stack)
+    error (_("No stack."));
+
+  count = get_number_trailer (&cmd, 0);
+
+  if (count < 0)
+    {
+      trailing = trailing_outermost_frame (-count);
+      count = -1;
+    }
+  else
+    trailing = get_current_frame ();
+
+  frame_apply_command_count ("frame apply", cmd, from_tty,
+			     trailing, count);
+}
+
+/* Implementation of the "faas" command.  */
+
+static void
+faas_command (const char *cmd, int from_tty)
+{
+  std::string expanded = std::string ("frame apply all -s ") + std::string (cmd);
+  execute_command (expanded.c_str (), from_tty);
+}
+
+
+/* Commands with a prefix of `frame'.  */
+struct cmd_list_element *frame_cmd_list = NULL;
+
 void
 _initialize_stack (void)
 {
+  static struct cmd_list_element *frame_apply_list = NULL;
+
   add_com ("return", class_stack, return_command, _("\
 Make selected stack frame return to its caller.\n\
 Control remains in the debugger, but when you continue\n\
@@ -2531,14 +2685,49 @@ An argument says how many frames down to go."));
 Same as the `down' command, but does not print anything.\n\
 This is useful in command scripts."));
 
-  add_com ("frame", class_stack, frame_command, _("\
+  add_prefix_cmd ("frame", class_stack, frame_command, _("\
 Select and print a stack frame.\nWith no argument, \
 print the selected stack frame.  (See also \"info frame\").\n\
 An argument specifies the frame to select.\n\
-It can be a stack frame number or the address of the frame."));
+It can be a stack frame number or the address of the frame."),
+		  &frame_cmd_list, "frame ", 1, &cmdlist);
 
   add_com_alias ("f", "frame", class_stack, 1);
 
+#define FRAME_APPLY_FLAGS_HELP "\
+FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
+  c(continue), s(silent).\n\
+Verbosity (default 2) controls what to print for a frame:\n\
+  0 : no frame info is printed\n\
+  1 : source line\n\
+  2 : location\n\
+  3 : location and address\n\
+  4 : source line and location\n\
+By default, if a COMMAND raises an error, frame apply is aborted.\n\
+Flag c indicates to print the error and continue.\n\
+Flag s indicates to silently ignore a COMMAND that raises an error\n\
+or produces no output."
+
+  add_prefix_cmd ("apply", class_stack, frame_apply_command,
+		  _("Apply a command to a number of frames.\n\
+Usage: frame apply COUNT [-FLAGS...] COMMAND\n\
+With a negative COUNT argument, applies the command on outermost -COUNT frames.\n"
+FRAME_APPLY_FLAGS_HELP),
+		  &frame_apply_list, "frame apply ", 1, &frame_cmd_list);
+
+  add_cmd ("all", class_stack, frame_apply_all_command,
+	   _("\
+Apply a command to all frames.\n\
+\n\
+Usage: frame apply all [-FLAGS...] COMMAND\n"
+FRAME_APPLY_FLAGS_HELP),
+	   &frame_apply_list);
+
+  add_com ("faas", class_stack, faas_command, _("\
+Apply a command to all frames (ignoring errors and empty output)\n\
+Usage: faas COMMAND\n\
+shortcut for 'frame apply all -s COMMAND'"));
+
   add_com_suppress_notification ("select-frame", class_stack, select_frame_command, _("\
 Select a stack frame without printing anything.\n\
 An argument specifies the frame to select.\n\
-- 
2.11.0

Re: [RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  5 May 2018 21:28:01 +0200
> 
> +  add_com ("faas", class_stack, faas_command, _("\
> +Apply a command to all frames (ignoring errors and empty output)\n\

Full sentence again.

Thanks.

Re: [RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND

[Simon Marchi]

On 2018-05-05 03:28 PM, Philippe Waroquiers wrote:
> Also implement the command 'faas COMMAND', a shortcut for
> 'frame apply all -s COMMAND'
> ---
>  gdb/stack.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 214 insertions(+), 25 deletions(-)
> 
> diff --git a/gdb/stack.c b/gdb/stack.c
> index ecf1ee8379..61e86ab18b 100644
> --- a/gdb/stack.c
> +++ b/gdb/stack.c
> @@ -777,7 +777,7 @@ do_gdb_disassembly (struct gdbarch *gdbarch,
>     
>     SRC_LINE: Print only source line.
>     LOCATION: Print only location.
> -   LOC_AND_SRC: Print location and source line.
> +   SRC_AND_LOC: Print location and source line.

Can you push this as a separate, obvious patch?

>  
>     Used in "where" output, and to emit breakpoint or step
>     messages.  */
> @@ -1687,6 +1687,39 @@ info_frame_command (const char *addr_exp, int from_tty)
>    }
>  }
>  
> +/* COUNT specifies the nr of outermost frames we are interested in.

nr -> number.  But actually I think the description is clear enough with only
the second sentence:

/* Return the starting frame needed to handle the COUNT outermost frames.  */

> +   trailing_outermost_frame returns the starting frame
> +   needed to handle COUNT outermost frames.  */
> +
> +static struct frame_info*

Space before *.

> +trailing_outermost_frame (int count)
> +{
> +  struct frame_info *current;
> +  struct frame_info *trailing;
> +
> +  trailing = get_current_frame ();
> +
> +  gdb_assert (count > 0);
> +
> +  current = trailing;
> +  while (current && count--)

current != nullptr

> +    {
> +      QUIT;
> +      current = get_prev_frame (current);
> +    }
> +
> +  /* Will stop when CURRENT reaches the top of the stack.
> +     TRAILING will be COUNT below it.  */
> +  while (current)

current != nullptr

> +    {
> +      QUIT;
> +      trailing = get_prev_frame (trailing);
> +      current = get_prev_frame (current);
> +    }
> +
> +  return trailing;
> +}
> +
>  /* Print briefly all stack frames or just the innermost COUNT_EXP
>     frames.  */
>  
> @@ -1696,7 +1729,6 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
>  {
>    struct frame_info *fi;
>    int count;
> -  int i;
>    int py_start = 0, py_end = 0;
>    enum ext_lang_bt_status result = EXT_LANG_BT_ERROR;
>  
> @@ -1756,30 +1788,13 @@ backtrace_command_1 (const char *count_exp, frame_filter_flags flags,
>  
>        if (count_exp != NULL && count < 0)
>  	{
> -	  struct frame_info *current;
> -
> -	  count = -count;
> -
> -	  current = trailing;
> -	  while (current && count--)
> -	    {
> -	      QUIT;
> -	      current = get_prev_frame (current);
> -	    }
> -
> -	  /* Will stop when CURRENT reaches the top of the stack.
> -	     TRAILING will be COUNT below it.  */
> -	  while (current)
> -	    {
> -	      QUIT;
> -	      trailing = get_prev_frame (trailing);
> -	      current = get_prev_frame (current);
> -	    }
> -
> +	  trailing = trailing_outermost_frame (-count);
>  	  count = -1;
>  	}
> +      else
> +	trailing = get_current_frame ();

We now do

  trailing = get_current_frame ();

both before the if and in the else.  We just need one, I don't mind which one.

>  
> -      for (i = 0, fi = trailing; fi && count--; i++, fi = get_prev_frame (fi))
> +      for (fi = trailing; fi && count--; fi = get_prev_frame (fi))

Removing the i variable seems to be something you could push as an obvious patch.

>  	{
>  	  QUIT;
>  
> @@ -2506,9 +2521,148 @@ func_command (const char *arg, int from_tty)
>      select_and_print_frame (frame);
>  }
>  
> +/* Apply a GDB command to a all stack frames, or innermost COUNT frames.
> +   With a negative COUNT, apply command on outermost -COUNT frames.
> +
> +   frame apply 3 info frame     Apply 'info frame' to frames 0, 1, 2
> +   frame apply -3 info frame    Apply 'info frame' to outermost 3 frames.
> +   frame apply all x/i $pc      Apply 'x/i $pc' cmd to all frames.
> +   frame apply all -s p local_var_no_idea_in_which_frame
> +                If a frame has a local variable called
> +                local_var_no_idea_in_which_frame, print frame
> +                and value of local_var_no_idea_in_which_frame.
> +   frame apply all -sqq p local_var_no_idea_in_which_frame
> +                Same as before, but only print the variable value.  */
> +
> +/* Apply a GDB command to COUNT stack frames, starting at trailing.
> +   COUNT -1 means all frames starting at trailing.  WHICH_COMMAND is used

trailing -> TRAILING

> +   for error messages.  */
> +static void
> +frame_apply_command_count (const char* which_command,
> +			   const char *cmd, int from_tty,
> +			   struct frame_info *trailing, int count)
> +{
> +  int print_what_v = 2; /* Corresponding to LOCATION.  */
> +  enum print_what print_what[5] =
> +    {
> +      LOC_AND_ADDRESS, /* Should never be used, this is verbosity 0.  */
> +      SRC_LINE,
> +      LOCATION,
> +      LOC_AND_ADDRESS,
> +      SRC_AND_LOC
> +    };
> +  bool cont;
> +  bool silent;
> +  struct frame_info *fi;
> +
> +  if (cmd != NULL)
> +    check_for_flags_vqcs (which_command, &cmd,
> +			  &print_what_v, 4,
> +			  &cont, &silent);
> +
> +  if (cmd == NULL || *cmd == '\000')

'\0' ?

> +    error (_("Please specify a command to apply on the selected frames"));
> +
> +  for (fi = trailing; fi && count--; fi = get_prev_frame (fi))
> +    {
> +      struct frame_id frame_id = get_frame_id (fi);
> +
> +      QUIT;
> +
> +      select_frame (fi);
> +      TRY
> +	{
> +	  std::string cmd_result = execute_command_to_string (cmd, from_tty);
> +	  if (!silent || cmd_result.length() > 0)
> +	    {
> +	      if (print_what_v > 0)
> +		print_stack_frame (fi, 1, print_what[print_what_v], 0);
> +	      printf_filtered ("%s", cmd_result.c_str ());
> +	    }
> +	}
> +      CATCH (ex, RETURN_MASK_ERROR)
> +	{
> +	  if (!silent)
> +	    {
> +	      if (print_what_v > 0)
> +		print_stack_frame (fi, 1, print_what [print_what_v], 0);
> +	      if (cont)
> +		printf_filtered ("%s\n", ex.message);
> +	      else
> +		throw_exception (ex);
> +	    }
> +	}
> +      END_CATCH;
> +
> +      /* execute_command_to_string might invalidate FI.  */
> +      fi = frame_find_by_id (frame_id);
> +      if (fi == NULL)
> +	{
> +	  trailing = NULL;
> +	  warning (_("Unable to restore previously selected frame."));
> +	  break;
> +	}
> +    }
> +}
> +
> +static void
> +frame_apply_all_command (const char *cmd, int from_tty)
> +{
> +  struct frame_info *fi;
> +  struct frame_info *trailing = NULL;
> +  int count;
> +  int i;

These variables are unused.

> +
> +  if (!target_has_stack)
> +    error (_("No stack."));
> +
> +  frame_apply_command_count ("frame apply all", cmd, from_tty,
> +			     get_current_frame(), INT_MAX);

Space after get_current_frame.

> +}
> +
> +/* Implementation of the "frame apply" command.  */
> +
> +static void
> +frame_apply_command (const char* cmd, int from_tty)
> +{
> +  int count;
> +  struct frame_info *trailing;
> +
> +  if (!target_has_stack)
> +    error (_("No stack."));
> +
> +  count = get_number_trailer (&cmd, 0);
> +
> +  if (count < 0)
> +    {
> +      trailing = trailing_outermost_frame (-count);
> +      count = -1;
> +    }
> +  else
> +    trailing = get_current_frame ();
> +
> +  frame_apply_command_count ("frame apply", cmd, from_tty,
> +			     trailing, count);
> +}

While testing this, I intuitively expected that this command would
work the same way as thread apply: take a frame number of frame number
range.  I had to think for a second when this did not do as I expected:

(gdb) bt
#0  break_here () at test.c:6
#1  0x000055555555461a in main (argc=1, argv=0x7fffffffde78) at test.c:12
(gdb) frame apply 0 print 123
(gdb) frame apply 1 print 123
#0  break_here () at test.c:6
$6 = 123

So I'm wondering whether it would make more sense to have both thread apply
and frame apply work in a similar way, to avoid confusing users.

> +
> +/* Implementation of the "faas" command.  */
> +
> +static void
> +faas_command (const char *cmd, int from_tty)
> +{
> +  std::string expanded = std::string ("frame apply all -s ") + std::string (cmd);
> +  execute_command (expanded.c_str (), from_tty);
> +}
> +
> +
> +/* Commands with a prefix of `frame'.  */
> +struct cmd_list_element *frame_cmd_list = NULL;
> +
>  void
>  _initialize_stack (void)
>  {
> +  static struct cmd_list_element *frame_apply_list = NULL;
> +
>    add_com ("return", class_stack, return_command, _("\
>  Make selected stack frame return to its caller.\n\
>  Control remains in the debugger, but when you continue\n\
> @@ -2531,14 +2685,49 @@ An argument says how many frames down to go."));
>  Same as the `down' command, but does not print anything.\n\
>  This is useful in command scripts."));
>  
> -  add_com ("frame", class_stack, frame_command, _("\
> +  add_prefix_cmd ("frame", class_stack, frame_command, _("\
>  Select and print a stack frame.\nWith no argument, \
>  print the selected stack frame.  (See also \"info frame\").\n\
>  An argument specifies the frame to select.\n\
> -It can be a stack frame number or the address of the frame."));
> +It can be a stack frame number or the address of the frame."),
> +		  &frame_cmd_list, "frame ", 1, &cmdlist);
>  
>    add_com_alias ("f", "frame", class_stack, 1);
>  
> +#define FRAME_APPLY_FLAGS_HELP "\
> +FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
> +  c(continue), s(silent).\n\
> +Verbosity (default 2) controls what to print for a frame:\n\
> +  0 : no frame info is printed\n\
> +  1 : source line\n\
> +  2 : location\n\
> +  3 : location and address\n\
> +  4 : source line and location\n\
> +By default, if a COMMAND raises an error, frame apply is aborted.\n\
> +Flag c indicates to print the error and continue.\n\
> +Flag s indicates to silently ignore a COMMAND that raises an error\n\
> +or produces no output."

I would prefer if this was a const char [] rather than a macro.

Thanks,

Simon

Re: [RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND

[Philippe Waroquiers]

On Thu, 2018-05-17 at 21:55 -0400, Simon Marchi wrote:
> > +/* Implementation of the "frame apply" command.  */
> > +
> > +static void
> > +frame_apply_command (const char* cmd, int from_tty)
> > +{
> > +  int count;
> > +  struct frame_info *trailing;
> > +
> > +  if (!target_has_stack)
> > +    error (_("No stack."));
> > +
> > +  count = get_number_trailer (&cmd, 0);
> > +
> > +  if (count < 0)
> > +    {
> > +      trailing = trailing_outermost_frame (-count);
> > +      count = -1;
> > +    }
> > +  else
> > +    trailing = get_current_frame ();
> > +
> > +  frame_apply_command_count ("frame apply", cmd, from_tty,
> > +			     trailing, count);
> > +}
> 
> While testing this, I intuitively expected that this command would
> work the same way as thread apply: take a frame number of frame number
> range.  I had to think for a second when this did not do as I expected:
> 
> (gdb) bt
> #0  break_here () at test.c:6
> #1  0x000055555555461a in main (argc=1, argv=0x7fffffffde78) at test.c:12
> (gdb) frame apply 0 print 123
> (gdb) frame apply 1 print 123
> #0  break_here () at test.c:6
> $6 = 123
> 
> So I'm wondering whether it would make more sense to have both thread apply
> and frame apply work in a similar way, to avoid confusing users.
When I started implementing this, I first intended to have a list
of frames, or range of frames (i.e. similar to thread apply).
However, I finally did the COUNT | -COUNT approach for the following
reasons:
  * this is similar to the backtrace command
  * the most common use cases are to show all frames,
     or some innermost frames, or some outermost frames.
    The use case of showing some 'middle frames range' is not a likely
    use case (as shown by the backtrace command, that does not have
    such feature). Similarly, showing 'cherry picked frames' seems
    also unlikely to be useful.
  * but the real difficulty is what range syntax to use to indicate
    either the innermost N frames or the outermost N frames ?
    For the innermost, we could use e.g. 
         frame apply 0-3 p $sp
      to show $sp for frame 0, 1, 2, 3.
    But what would be the syntax to print $sp for e.g. 5
    outermost frames ?
    It is difficult for the user to use "absolute" numbers as
    the frame numbering will vary, depending on the stack depth.
    And we also have to find a notation to indicate that the range
    is at the outermost side. I could not find a reasonable
    way to indicate this.
      e.g.   frame apply -0-4 p $sp
    looks an ugly way to indicate the outermost 5 frames.


So, in summary, COUNT | -COUNT has the advantage of being
compatible with backtrace/easy to understand.
Of course, having it similar to backtrace make it not looking
like frame apply. But IMO, that disadvantage is not major, compared
to the other syntax I thought to, which will in any case differ
from the thread apply syntax IMO.


> > +#define FRAME_APPLY_FLAGS_HELP "\
> > +FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
> > +  c(continue), s(silent).\n\
> > +Verbosity (default 2) controls what to print for a frame:\n\
> > +  0 : no frame info is printed\n\
> > +  1 : source line\n\
> > +  2 : location\n\
> > +  3 : location and address\n\
> > +  4 : source line and location\n\
> > +By default, if a COMMAND raises an error, frame apply is aborted.\n\
> > +Flag c indicates to print the error and continue.\n\
> > +Flag s indicates to silently ignore a COMMAND that raises an error\n\
> > +or produces no output."
> 
> I would prefer if this was a const char [] rather than a macro.
The macro is used to build a string literal. I quickly tried, but I found
no straightforward way to build a string literal using a factorised
const char [] value.

Philippe

Re: [RFC 2/5] Implement frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND

[Simon Marchi]

On 2018-05-05 03:28 PM, Philippe Waroquiers wrote:
> Also implement the command 'faas COMMAND', a shortcut for
> 'frame apply all -s COMMAND'

Should "frame apply" attempt to restore the originally selected frame,
like "thread apply" does for threads?

Simon

[RFC 4/5] Documentation changes for 'frame apply' and 'thread apply'

[Philippe Waroquiers]

Documents the new commands
   'frame apply'
   faas
   taas
   tfaas

Documents the new argument -FLAGS... added to 'thread apply'
---
 gdb/doc/gdb.texinfo | 215 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 212 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 28f083f96e..71f71e94a0 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2933,7 +2933,7 @@ programs:
 @item automatic notification of new threads
 @item @samp{thread @var{thread-id}}, a command to switch among threads
 @item @samp{info threads}, a command to inquire about existing threads
-@item @samp{thread apply [@var{thread-id-list}] [@var{all}] @var{args}},
+@item @samp{thread apply [@var{thread-id-list} | @code{all}] @var{args}},
 a command to apply a command to a list of threads
 @item thread-specific breakpoints
 @item @samp{set print thread-events}, which controls printing of 
@@ -3170,7 +3170,7 @@ threads.
 
 @kindex thread apply
 @cindex apply command to several threads
-@item thread apply [@var{thread-id-list} | all [-ascending]] @var{command}
+@item thread apply [@var{thread-id-list} | all [-ascending]] [@var{-flags@dots{}}] @var{command}
 The @code{thread apply} command allows you to apply the named
 @var{command} to one or more threads.  Specify the threads that you
 want affected using the thread ID list syntax (@pxref{thread ID
@@ -3179,6 +3179,53 @@ command to all threads in descending order, type @kbd{thread apply all
 @var{command}}.  To apply a command to all threads in ascending order,
 type @kbd{thread apply all -ascending @var{command}}.
 
+The @var{flags} control what output to produce and how to handle
+errors raised when applying @var{command} to a thread.  @var{flags}
+must start with a @code{-} directly followed by one or more letters in
+@code{vqcs}.
+
+@table @asis
+@item Flags @code{c} and @code{s} : error handling when applying @var{command}.
+By default, an error raised during the execution of a @var{command}
+will abort @code{thread apply}.  If the flag @code{c} (continue) is
+given, the error will be printed and the execution of @code{thread
+apply} is continued.  The flag @code{s} (silent) means to silently
+ignore a @var{command} execution that raises an error or produces an
+empty output.  The flags @code{c} and @code{s} cannot be given
+simultaneously.
+
+@item Flags @code{v} and @code{q} : change thread information printed when applying @var{command}.
+@noindent
+Some thread information is printed before the output or error produced
+by applying @var{command} to a thread.  The printed thread information
+is controlled by the verbosity.  A flag @code{v} increases the
+verbosity, a flag @code{q} decreases the verbosity.
+
+At verbosity 1 (the default value), the per-inferior thread number and
+target system's thread id are printed.  At verbosity 0, no thread info
+is printed.
+
+@end table
+
+@kindex taas
+@cindex apply command to all threads (ignoring errors and empty output)
+@item taas @var{command}
+Shortcut for @code{thread apply all -s @var{COMMAND}}.
+Applies @var{COMMAND} on all threads, ignoring errors and empty output.
+
+@kindex tfaas
+@cindex apply a command to all frames of all threads (ignoring errors and empty output)
+@item tfaas @var{command}
+Shortcut for @code{thread apply all -s frame apply all -s @var{COMMAND}}.
+Applies @var{COMMAND} on all frames of all threads, ignoring errors and empty output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the thread or frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) tfaas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
 
 @kindex thread name
 @cindex name a thread
@@ -7300,6 +7347,7 @@ currently executing frame and describes it briefly, similar to the
 * Backtrace::                   Backtraces
 * Selection::                   Selecting a frame
 * Frame Info::                  Information on a frame
+* Frame Apply::                 Applying a command to several frames
 * Frame Filter Management::     Managing frame filters
 
 @end menu
@@ -7712,6 +7760,167 @@ accessible at the point of execution of the selected frame.
 
 @end table
 
+@node Frame Apply
+@section Applying a Command to Several Frames.
+@kindex frame apply
+@cindex apply command to several frames
+@table @code
+@item frame apply [all | @var{count} | @var{-count} ] [@var{-flags@dots{}}] @var{command}
+The @code{frame apply} command allows you to apply the named
+@var{command} to one or more frames@footnote{Note that the frames on
+which @code{frame apply} applies a command are also influenced by the
+@code{set backtrace} settings such as @code{set backtrace past-main}
+and @code{set backtrace limit N}. See @ref{Backtrace,,Backtraces}}.
+
+@table @code
+@item @code{all}
+Specify @code{all} to apply @var{command} to all frames.
+
+@item @var{count}
+Use @var{count} to apply @var{command} to the innermost @var{count}
+frames, where @var{count} is a positive number.
+
+@item @var{-count}
+Use @var{-count} to apply @var{command} to the outermost @var{count}
+frames, where @var{count} is a positive number.
+@end table
+
+
+@end table
+
+The @var{flags} control what output to produce and how to handle
+errors raised when applying @var{command} to a frame.  @var{flags}
+must start with a @code{-} directly followed by one or more letters in
+@code{vqcs}.
+
+@table @asis
+@item Flags @code{c} and @code{s} : error handling when applying @var{command}.
+By default, an error raised during the execution of a @var{command}
+will abort @code{frame apply}.  If the flag @code{c} (continue) is
+given, the error will be printed and the execution of @code{frame
+apply} is continued.  The flag @code{s} (silent) means to silently
+ignore a @var{command} execution that raises an error or produces an
+empty output.  The flags @code{c} and @code{s} cannot be given
+simultaneously.
+
+
+The following example shows how these flags are working when applying
+the command @code{p j} to all frames, where variable @code{j} can only
+be successfully printed in the outermost @code{#1 main} frame.
+
+@smallexample
+@group
+(gdb) frame apply all p j
+#0  some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+(gdb) frame apply all -c p j
+#0  some_function (i=5) at fun.c:4
+No symbol "j" in current context.
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$1 = 5
+(gdb) frame apply all -s p j
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$2 = 5
+(gdb)
+@end group
+@end smallexample
+
+@item Flags @code{v} and @code{q} : change frame information printed when applying @var{command}.
+@noindent
+Some frame information is printed before the output or error produced
+by applying @var{command} to a frame.  The printed frame information
+@footnote{Note that the way the frame information is printed can be changed
+using the @code{set backtrace} settings such as @code{set
+filename-display}. @ref{Backtrace,,Backtraces}}
+is controlled by the verbosity.   A flag @code{v} increases the
+verbosity, a flag @code{q} decreases the verbosity.
+
+At verbosity 2 (the default value), the location is printed before
+the command output:
+
+@smallexample
+@group
+(gdb) frame apply all p $sp
+#0  some_function (i=5) at fun.c:4
+$4 = (void *) 0xffffd1e0
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$5 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 3, the location and address is printed:
+
+@smallexample
+@group
+(gdb) frame apply all -v p $sp
+#0  0x565555b1 in some_function (i=5) at fun.c:4
+$6 = (void *) 0xffffd1e0
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+$7 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 4, the source line and address is printed:
+@smallexample
+@group
+(gdb) frame apply all -vv p $sp
+#0  some_function (i=5) at fun.c:4
+4	   printf ("value of i is %d\n", i);
+$8 = (void *) 0xffffd1e0
+#1  0x565555fb in main (argc=1, argv=0xffffd2c4) at fun.c:11
+11	   some_function (j);
+$9 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 1, only the source line is printed:
+
+@smallexample
+@group
+(gdb) frame apply all -q p $sp
+4	   printf ("value of i is %d\n", i);
+$10 = (void *) 0xffffd1e0
+0x565555fb	11	   some_function (j);
+$11 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+At verbosity 0, no frame information is printed:
+@smallexample
+@group
+(gdb) frame apply all -qq p $sp
+$12 = (void *) 0xffffd1e0
+$13 = (void *) 0xffffd1f0
+(gdb)
+@end group
+@end smallexample
+
+@end table
+
+@table @code
+
+@kindex faas
+@cindex apply a command to all frames (ignoring errors and empty output)
+@item faas @var{command}
+Shortcut for @code{frame apply all -s @var{COMMAND}}.
+Applies @var{COMMAND} on all frames, ignoring errors and empty output.
+
+It can for example be used to print a local variable or a function
+argument without knowing the frame where this variable or argument
+is, using:
+@smallexample
+(@value{GDBP}) faas p some_local_var_i_do_not_remember_where_it_is
+@end smallexample
+
+Note that the command @code{tfaas @var{command}} applies @var{command}
+on all frames of all threads. @ref{Threads,,Threads}
+@end table
+
+
 @node Frame Filter Management
 @section Management of Frame Filters.
 @cindex managing frame filters
@@ -11090,7 +11299,7 @@ Visiting node of type NODE_INTEGER
 convenience functions.
 
 @table @code
-@item help function
+@item help functionu
 @kindex help function
 @cindex show all convenience functions
 Print a list of all convenience functions.
-- 
2.11.0

Re: [RFC 4/5] Documentation changes for 'frame apply' and 'thread apply'

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  5 May 2018 21:28:03 +0200
> 
> +@table @asis

Since all the @item's in this table use @code, it is better to say

  @table @code

> +@item Flags @code{c} and @code{s} : error handling when applying @var{command}.

This is not a good way of formatting a table.  I suggest to use this
instead:

 By default, @value{GDBN} displays some thread information before the
 output produced by @var{command}, and an error raised during the
 execution of a @var{command} will abort @code{thread apply}.  The
 following flags can be used to fine-tune this behavior:

 @table @code
 @item c
 The flag @code{c}, which stands for @samp{continue}, causes any
 errors in @var{command} to be displayed, and the execution of
 @vode{thread apply} then continues.
 @item s
 The flag @code{s}, which stands for @samp{silent}, causes any errors
 or empty output produced by a @var{command} to be silently ignored.
 That is, the execution continues, but the errors are not printed.
 @item v
 The flag @code{v} increases the verbosity of the info displayed for
 each thread.
 @item q
 The flag @code{q} (@samp{quiet}) decreases the verbosity.
 @end @table

 Flags @code{c} and @code{s} cannot be used together.

 The default value of verbosity, 1, prints the per-inferior thread
 number and the target system's thread ID.  Under verbosity 0, no
 thread info is printed.

> +@item tfaas @var{command}
> +Shortcut for @code{thread apply all -s frame apply all -s @var{COMMAND}}.

Any reason why -s should be specified twice?

> +The @var{flags} control what output to produce and how to handle
> +errors raised when applying @var{command} to a frame.  @var{flags}
> +must start with a @code{-} directly followed by one or more letters in
> +@code{vqcs}.

I suggest to use here the same paradigm as shown above for "thread
apply".

> +Some frame information is printed before the output or error produced
> +by applying @var{command} to a frame.  The printed frame information
> +@footnote{Note that the way the frame information is printed can be changed
> +using the @code{set backtrace} settings such as @code{set
> +filename-display}. @ref{Backtrace,,Backtraces}}

Please use @xref here, and leave 2 spaces between sentences.  (And I'm
not sure it's a good idea to have cross-references in footnotes.  Did
you make sure this produces reasonable results in the printed manual?)

> +Note that the command @code{tfaas @var{command}} applies @var{command}
> +on all frames of all threads. @ref{Threads,,Threads}

Ditto.

Thanks.

[RFC 1/5] Add helper functions check_for_flags and check_for_flags_vqcs

[Philippe Waroquiers]

check_for_flags helper function allows to look for a set of flags at
the start of a string.

check_for_flags_vqcs is a specialised helper function to handle
the flags vqcs, that are used in the new command 'frame apply'
and in the command 'thread apply.

Modify number_or_range_parser::get_number to differentiate a
- followed by digits from a - followed by a flag (i.e. an alpha).
That is needed for the addition of the optional -FLAGS... argument to
thread apply ID... [-FLAGS...] COMMAND

Modify gdb/testsuite/gdb.multi/tids.exp, as the gdb error message
for 'thread apply -$unknownconvvar p 1'
is now the more clear:
  Invalid thread ID: -$unknownconvvar p 1
instead of previously:
  negative value
---
 gdb/cli/cli-utils.c              | 116 ++++++++++++++++++++++++++++++++++++++-
 gdb/cli/cli-utils.h              |  30 ++++++++++
 gdb/testsuite/gdb.multi/tids.exp |   4 +-
 3 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index c55b5035e4..4abd501d52 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -22,7 +22,6 @@
 #include "value.h"
 
 #include <ctype.h>
-
 /* See documentation in cli-utils.h.  */
 
 int
@@ -169,7 +168,10 @@ number_or_range_parser::get_number ()
       /* Default case: state->m_cur_tok is pointing either to a solo
 	 number, or to the first number of a range.  */
       m_last_retval = get_number_trailer (&m_cur_tok, '-');
-      if (*m_cur_tok == '-')
+      /* if get_number_trailer has found a -, it might be the start
+	 of flags. So, do not parse a range if the - is followed by
+	 an alpha.  */
+      if (*m_cur_tok == '-' && !isalpha(*(m_cur_tok+1)))
 	{
 	  const char **temp;
 
@@ -196,7 +198,10 @@ number_or_range_parser::get_number ()
 	}
     }
   else
-    error (_("negative value"));
+    {
+      if (*m_cur_tok >= '0' && *m_cur_tok <= '9')
+	error (_("negative value"));
+    }
   m_finished = *m_cur_tok == '\0';
   return m_last_retval;
 }
@@ -304,3 +309,108 @@ check_for_argument (const char **str, const char *arg, int arg_len)
     }
   return 0;
 }
+
+int
+check_for_flags (const char **str, const char *flags,
+		 int *flags_counts)
+{
+  const char *p = *str;
+  bool all_valid = true;
+
+  /* First set the flags_counts to 0.  */
+  {
+    const char *f = flags;
+    while (*f)
+      {
+	flags_counts[f - flags] = 0;
+	f++;
+      }
+  }
+
+  if (*p != '-')
+    return 0;
+
+  p++;
+  /* If - is the last char, or is followed by a space or a $, then
+     we do not have flags.  */
+  if (*p == '\0' || (isspace (*p) || *p == '$'))
+    return 0;
+
+  /* We might have a command that accepts optional flags followed by
+     a negative integer. So, do not handle a negative integer as invalid
+     flags : rather let the caller handle the negative integer.  */
+  {
+    const char *p1 = p;
+    while (*p1 >= '0' && *p1 <= '9')
+      ++p1;
+    if (p != p1 && (*p1 == '\0' || isspace (*p1)))
+      return 0;
+  }
+
+  /* We have a set of flags :
+     Scan and validate the flags, and update flags_counts for valid flags.  */
+  while (*p != '\0' && !isspace (*p))
+    {
+      const char *f = flags;
+      bool valid = false;
+
+      while (*f && !valid)
+	{
+	  valid = *f == *p;
+	  if (valid)
+	    {
+	      flags_counts[f - flags]++;
+	      break;
+	    }
+	  f++;
+	}
+      all_valid = all_valid && valid;
+      p++;
+    }
+
+  /* Skip the space(s) */
+  while (*p && isspace((int) *p))
+    ++p;
+
+  if (all_valid)
+    {
+      *str = p;
+      return 1;
+    }
+  else
+    return -1;
+}
+
+int
+check_for_flags_vqcs (const char *which_command, const char **str,
+		      int *print_what_v, int max_v,
+		      bool *cont, bool *silent)
+{
+  const char *flags = "vqcs";
+  int flags_counts[4];
+  int res;
+
+  *cont = false;
+  *silent = false;
+
+  res = check_for_flags (str, flags, flags_counts);
+  if (res == 0)
+    return 0;
+  if (res == -1)
+    error (_("%s only accepts flags %s"), which_command, flags);
+  gdb_assert (res == 1);
+
+  *print_what_v = *print_what_v + flags_counts[0] - flags_counts[1];
+  if (*print_what_v < 0)
+    *print_what_v = 0;
+  if (*print_what_v > max_v)
+    *print_what_v = max_v;
+
+  *cont = flags_counts[2] > 0;
+  *silent = flags_counts[3] > 0;
+  if (*cont && *silent)
+    error (_("%s does not accept at the same time flags c and s"),
+	   which_command);
+
+  return res;
+}
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index e34ee0df37..defbedf980 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -173,4 +173,34 @@ check_for_argument (char **str, const char *arg, int arg_len)
 			     arg, arg_len);
 }
 
+/* A helper function that looks for a set of flags at the start of a
+   string.  The possible flags are given as a null terminated string.
+   The flags must also either be at the end of the string,
+   or be followed by whitespace.  Returns 1 if it finds only valid flags,
+   0 if no flags are found, -1 if invalid flags are found.
+   FLAGS_COUNTS must be an int array of length equal to strlen(flags).
+   Sets *FLAGS_COUNTS to the number of times the corresponding flag
+   was found in *STR.
+   If only valid flags are found, it updates *STR.  Note that a negative
+   integer (e.g. -123) will not be considered as an invalid set of flags.  */
+extern int check_for_flags (const char **str, const char *flags,
+			    int *flags_counts);
+
+/* A helper function that uses check_for_flags to handle the flags vqcs :
+     A flag v increases *print_what_v.
+     A flag q decreases *print_what_v.
+     A flag c sets *cont to true, otherwise to false.
+     A flag s sets *silent to true, otherwise to false.
+   The caller must initialise *PRINT_WHAT_V to the default verbosity.
+   MAX_V gives the maximum verbosity : the value returned in *PRINT_WHAT_V
+   will always be >= 0 and <= MAX_V.
+   WHICH_COMMAND is used in the error message in case invalid flags are found.
+
+   Returns 1 and updates *STR if it finds only valid flags.
+   Returns 0 if no flags are found.
+   Raises an error if invalid flags are found.
+*/
+extern int check_for_flags_vqcs (const char *which_command, const char **str,
+				 int *print_what_v, int max_v,
+				 bool *cont, bool *silent);
 #endif /* CLI_UTILS_H */
diff --git a/gdb/testsuite/gdb.multi/tids.exp b/gdb/testsuite/gdb.multi/tids.exp
index 67349b5759..ae8328d997 100644
--- a/gdb/testsuite/gdb.multi/tids.exp
+++ b/gdb/testsuite/gdb.multi/tids.exp
@@ -350,8 +350,8 @@ with_test_prefix "two inferiors" {
 	thr_apply_info_thr_error "${prefix}1-" "inverted range"
 	thr_apply_info_thr_error "${prefix}2-1" "inverted range"
 	thr_apply_info_thr_error "${prefix}2-\$one" "inverted range"
-	thr_apply_info_thr_error "${prefix}-1" "negative value"
-	thr_apply_info_thr_error "${prefix}-\$one" "negative value"
+	thr_apply_info_thr_error "${prefix}-1" "Invalid thread ID: ${prefix_re}-1"
+	thr_apply_info_thr_error "${prefix}-\$one" "Invalid thread ID: ${prefix_re}-\\\$one"
 	thr_apply_info_thr_error "${prefix}\$minus_one" \
 	    "negative value: ${prefix_re}\\\$minus_one"
 
-- 
2.11.0

Re: [RFC 1/5] Add helper functions check_for_flags and check_for_flags_vqcs

[Simon Marchi]

Hi Philippe,

I don't see the full picture yet just with this patch, so here is a
first pass of comments, more nits than anything else.

On 2018-05-05 03:28 PM, Philippe Waroquiers wrote:
> check_for_flags helper function allows to look for a set of flags at
> the start of a string.
> 
> check_for_flags_vqcs is a specialised helper function to handle
> the flags vqcs, that are used in the new command 'frame apply'
> and in the command 'thread apply.
> 
> Modify number_or_range_parser::get_number to differentiate a
> - followed by digits from a - followed by a flag (i.e. an alpha).
> That is needed for the addition of the optional -FLAGS... argument to
> thread apply ID... [-FLAGS...] COMMAND
> 
> Modify gdb/testsuite/gdb.multi/tids.exp, as the gdb error message
> for 'thread apply -$unknownconvvar p 1'
> is now the more clear:
>   Invalid thread ID: -$unknownconvvar p 1
> instead of previously:
>   negative value
> ---
>  gdb/cli/cli-utils.c              | 116 ++++++++++++++++++++++++++++++++++++++-
>  gdb/cli/cli-utils.h              |  30 ++++++++++
>  gdb/testsuite/gdb.multi/tids.exp |   4 +-
>  3 files changed, 145 insertions(+), 5 deletions(-)
> 
> diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
> index c55b5035e4..4abd501d52 100644
> --- a/gdb/cli/cli-utils.c
> +++ b/gdb/cli/cli-utils.c
> @@ -22,7 +22,6 @@
>  #include "value.h"
>  
>  #include <ctype.h>
> -

Unintended change?

>  /* See documentation in cli-utils.h.  */
>  
>  int
> @@ -169,7 +168,10 @@ number_or_range_parser::get_number ()
>        /* Default case: state->m_cur_tok is pointing either to a solo
>  	 number, or to the first number of a range.  */
>        m_last_retval = get_number_trailer (&m_cur_tok, '-');
> -      if (*m_cur_tok == '-')
> +      /* if get_number_trailer has found a -, it might be the start

Capital "i"

> +	 of flags. So, do not parse a range if the - is followed by
> +	 an alpha.  */
> +      if (*m_cur_tok == '-' && !isalpha(*(m_cur_tok+1)))

Missing space after isalpha, spaces around +.  It could also be written
m_cur_token[1].

Could you add some simple unit tests in the unittests/ directory for
number_or_range_parser (I don't think there are any so far)?  It will
help illustrate your use case.

>  	{
>  	  const char **temp;
>  
> @@ -196,7 +198,10 @@ number_or_range_parser::get_number ()
>  	}
>      }
>    else
> -    error (_("negative value"));
> +    {
> +      if (*m_cur_tok >= '0' && *m_cur_tok <= '9')

Use isdigit?

> +	error (_("negative value"));
> +    }
>    m_finished = *m_cur_tok == '\0';
>    return m_last_retval;
>  }
> @@ -304,3 +309,108 @@ check_for_argument (const char **str, const char *arg, int arg_len)
>      }
>    return 0;
>  }
> +

Add a

  /* See cli-utils.h.  */

> +int
> +check_for_flags (const char **str, const char *flags,
> +		 int *flags_counts)
> +{
> +  const char *p = *str;
> +  bool all_valid = true;
> +
> +  /* First set the flags_counts to 0.  */
> +  {
> +    const char *f = flags;
> +    while (*f)
> +      {
> +	flags_counts[f - flags] = 0;
> +	f++;
> +      }
> +  }

What about something like

  memset (flags_count, 0, sizeof (flags_count[0]) * strlen (flags));

> +
> +  if (*p != '-')
> +    return 0;
> +
> +  p++;
> +  /* If - is the last char, or is followed by a space or a $, then
> +     we do not have flags.  */
> +  if (*p == '\0' || (isspace (*p) || *p == '$'))

You can remove some parenthesis:

  if (*p == '\0' || isspace (*p) || *p == '$')

But could we be more restrictive here and only allow isalpha characters?

  if (!isalpha (*p))
    return 0;

> +    return 0;
> +
> +  /* We might have a command that accepts optional flags followed by
> +     a negative integer. So, do not handle a negative integer as invalid
> +     flags : rather let the caller handle the negative integer.  */
> +  {
> +    const char *p1 = p;
> +    while (*p1 >= '0' && *p1 <= '9')

isdigit?

> +      ++p1;
> +    if (p != p1 && (*p1 == '\0' || isspace (*p1)))
> +      return 0;
> +  }
> +
> +  /* We have a set of flags :
> +     Scan and validate the flags, and update flags_counts for valid flags.  */
> +  while (*p != '\0' && !isspace (*p))
> +    {
> +      const char *f = flags;
> +      bool valid = false;
> +
> +      while (*f && !valid)

*f != nullptr

If I understand the algorithm right, I think it is not necessary to check for
!valid here.

> +	{
> +	  valid = *f == *p;
> +	  if (valid)
> +	    {
> +	      flags_counts[f - flags]++;
> +	      break;
> +	    }
> +	  f++;
> +	}
> +      all_valid = all_valid && valid;
> +      p++;
> +    }
> +
> +  /* Skip the space(s) */
> +  while (*p && isspace((int) *p))

Space after isspace, but it could probably use the skip_spaces function.

> +    ++p;
> +
> +  if (all_valid)
> +    {
> +      *str = p;
> +      return 1;
> +    }
> +  else
> +    return -1;
> +}
> +

/* See cli-utils.h.  */

> +int
> +check_for_flags_vqcs (const char *which_command, const char **str,
> +		      int *print_what_v, int max_v,
> +		      bool *cont, bool *silent)
> +{
> +  const char *flags = "vqcs";
> +  int flags_counts[4];

Maybe use strlen (flags) instead of the literal 4?

> +  int res;
> +
> +  *cont = false;
> +  *silent = false;
> +
> +  res = check_for_flags (str, flags, flags_counts);
> +  if (res == 0)
> +    return 0;
> +  if (res == -1)
> +    error (_("%s only accepts flags %s"), which_command, flags);
> +  gdb_assert (res == 1);
> +
> +  *print_what_v = *print_what_v + flags_counts[0] - flags_counts[1];
> +  if (*print_what_v < 0)
> +    *print_what_v = 0;
> +  if (*print_what_v > max_v)
> +    *print_what_v = max_v;
> +
> +  *cont = flags_counts[2] > 0;
> +  *silent = flags_counts[3] > 0;
> +  if (*cont && *silent)
> +    error (_("%s does not accept at the same time flags c and s"),
> +	   which_command);

Another common way to say this would be

  ""%s: flags c and s are mutually exclusive."

Thanks,

Simon

Re: [RFC 1/5] Add helper functions check_for_flags and check_for_flags_vqcs

[Philippe Waroquiers]

On Thu, 2018-05-17 at 21:22 -0400, Simon Marchi wrote:
> > +  /* First set the flags_counts to 0.  */
> > +  {
> > +    const char *f = flags;
> > +    while (*f)
> > +      {
> > +	flags_counts[f - flags] = 0;
> > +	f++;
> > +      }
> > +  }
> 
> What about something like
> 
>   memset (flags_count, 0, sizeof (flags_count[0]) * strlen (flags));

The code initialising the flags_counts is somewhat similar in structure
to the code that increments the flags_counts.

So, it looks more clear to me to have the zero-ing code and
the incrementing code looking like each other.

But if for gdb, using memset is the typical pattern to zero
an array of int, fine for me.

What do you think ?

Philippe

Re: [RFC 1/5] Add helper functions check_for_flags and check_for_flags_vqcs

[Simon Marchi]

On 2018-05-18 05:42 PM, Philippe Waroquiers wrote:
> On Thu, 2018-05-17 at 21:22 -0400, Simon Marchi wrote:
>>> +  /* First set the flags_counts to 0.  */
>>> +  {
>>> +    const char *f = flags;
>>> +    while (*f)
>>> +      {
>>> +	flags_counts[f - flags] = 0;
>>> +	f++;
>>> +      }
>>> +  }
>>
>> What about something like
>>
>>   memset (flags_count, 0, sizeof (flags_count[0]) * strlen (flags));
> 
> The code initialising the flags_counts is somewhat similar in structure
> to the code that increments the flags_counts.
> 
> So, it looks more clear to me to have the zero-ing code and
> the incrementing code looking like each other.
> 
> But if for gdb, using memset is the typical pattern to zero
> an array of int, fine for me.
> 
> What do you think ?

I don't really mind, I just thought the memset was short and clear,
but both work.

Simon

Re: [RFC 1/5] Add helper functions check_for_flags and check_for_flags_vqcs

[Philippe Waroquiers]

On Fri, 2018-05-18 at 19:38 -0400, Simon Marchi wrote:
> I don't really mind, I just thought the memset was short and clear,
> but both work.
Ok, I will use memset (clearly, the usual pattern).

Thanks for the feedback.
I will start working on the comments.

Philippe

[RFC 3/5] Add -FLAGS... argument to thread apply

[Philippe Waroquiers]

Also implements 2 new commands:
taas COMMAND
   shortcut for 'thread apply all -s COMMAND'
tfaas COMMAND
   shortcut for 'thread apply all -s frame apply all -s COMMAND'
---
 gdb/thread.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 108 insertions(+), 20 deletions(-)

diff --git a/gdb/thread.c b/gdb/thread.c
index a09d7e0ba0..2fa075d4b1 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1581,6 +1581,41 @@ tp_array_compar (const thread_info *a, const thread_info *b)
     return (a->per_inf_num > b->per_inf_num);
 }
 
+static void
+thr_try_catch_cmd (thread_info *thr, const char *cmd, int from_tty,
+		   int print_what_v,
+		   bool cont, bool silent)
+{
+  switch_to_thread (thr->ptid);
+  TRY
+    {
+      std::string cmd_result = execute_command_to_string (cmd, from_tty);
+      if (!silent || cmd_result.length() > 0)
+	{
+	  if (print_what_v > 0)
+	    printf_filtered (_("\nThread %s (%s):\n"),
+			     print_thread_id (thr),
+			     target_pid_to_str (inferior_ptid));
+	  printf_filtered ("%s", cmd_result.c_str ());
+	}
+    }
+  CATCH (ex, RETURN_MASK_ERROR)
+    {
+      if (!silent)
+	{
+	  if (print_what_v > 0)
+	    printf_filtered (_("\nThread %s (%s):\n"),
+			     print_thread_id (thr),
+			     target_pid_to_str (inferior_ptid));
+	  if (cont)
+	    printf_filtered ("%s\n", ex.message);
+	  else
+	    throw_exception (ex);
+	}
+    }
+  END_CATCH;
+}
+
 /* Apply a GDB command to a list of threads.  List syntax is a whitespace
    separated list of numbers, or ranges, or the keyword `all'.  Ranges consist
    of two numbers separated by a hyphen.  Examples:
@@ -1592,6 +1627,10 @@ tp_array_compar (const thread_info *a, const thread_info *b)
 static void
 thread_apply_all_command (const char *cmd, int from_tty)
 {
+  int print_what_v = 1; /* Print thread id/thread/lwp.  */
+  bool cont;
+  bool silent;
+
   tp_array_compar_ascending = false;
   if (cmd != NULL
       && check_for_argument (&cmd, "-ascending", strlen ("-ascending")))
@@ -1600,8 +1639,13 @@ thread_apply_all_command (const char *cmd, int from_tty)
       tp_array_compar_ascending = true;
     }
 
+  if (cmd != NULL)
+    check_for_flags_vqcs ("thread apply all", &cmd,
+			  &print_what_v, 1,
+			  &cont, &silent);
+
   if (cmd == NULL || *cmd == '\000')
-    error (_("Please specify a command following the thread ID list"));
+    error (_("Please specify a command at the end of thread apply all"));
 
   update_thread_list ();
 
@@ -1637,14 +1681,9 @@ thread_apply_all_command (const char *cmd, int from_tty)
 
       for (thread_info *thr : thr_list_cpy)
 	if (thread_alive (thr))
-	  {
-	    switch_to_thread (thr->ptid);
-	    printf_filtered (_("\nThread %s (%s):\n"),
-			     print_thread_id (thr),
-			     target_pid_to_str (inferior_ptid));
-
-	    execute_command (cmd, from_tty);
-	  }
+	  thr_try_catch_cmd (thr, cmd, from_tty,
+			     print_what_v,
+			     cont, silent);
     }
 }
 
@@ -1653,7 +1692,11 @@ thread_apply_all_command (const char *cmd, int from_tty)
 static void
 thread_apply_command (const char *tidlist, int from_tty)
 {
+  int print_what_v = 1; /* Print thread id/thread/lwp.  */
+  bool cont;
+  bool silent;
   const char *cmd = NULL;
+  const char *cmd_or_flags;
   tid_range_parser parser;
 
   if (tidlist == NULL || *tidlist == '\000')
@@ -1671,6 +1714,12 @@ thread_apply_command (const char *tidlist, int from_tty)
 	}
     }
 
+  cmd_or_flags = cmd;
+  if (cmd != NULL)
+    check_for_flags_vqcs ("thread apply", &cmd,
+			  &print_what_v, 1,
+			  &cont, &silent);
+
   if (cmd == NULL)
     error (_("Please specify a command following the thread ID list"));
 
@@ -1680,7 +1729,7 @@ thread_apply_command (const char *tidlist, int from_tty)
   scoped_restore_current_thread restore_thread;
 
   parser.init (tidlist, current_inferior ()->num);
-  while (!parser.finished () && parser.cur_tok () < cmd)
+  while (!parser.finished () && parser.cur_tok () < cmd_or_flags)
     {
       struct thread_info *tp = NULL;
       struct inferior *inf;
@@ -1725,14 +1774,31 @@ thread_apply_command (const char *tidlist, int from_tty)
 	  continue;
 	}
 
-      switch_to_thread (tp->ptid);
-
-      printf_filtered (_("\nThread %s (%s):\n"), print_thread_id (tp),
-		       target_pid_to_str (inferior_ptid));
-      execute_command (cmd, from_tty);
+      thr_try_catch_cmd (tp, cmd, from_tty,
+			 print_what_v,
+			 cont, silent);
     }
 }
 
+
+/* Implementation of the "taas" command.  */
+
+static void
+taas_command (const char *cmd, int from_tty)
+{
+  std::string expanded = std::string ("thread apply all -s ") + std::string (cmd);
+  execute_command (expanded.c_str (), from_tty);
+}
+
+/* Implementation of the "tfaas" command.  */
+
+static void
+tfaas_command (const char *cmd, int from_tty)
+{
+  std::string expanded = std::string ("thread apply all -s frame apply all -s ") + std::string (cmd);
+  execute_command (expanded.c_str (), from_tty);
+}
+
 /* Switch to the specified thread, or print the current thread.  */
 
 void
@@ -2027,22 +2093,44 @@ Use this command to switch between threads.\n\
 The new thread ID must be currently known."),
 		  &thread_cmd_list, "thread ", 1, &cmdlist);
 
+#define THREAD_APPLY_FLAGS_HELP "\
+FLAGS letters are v(increase verbosity), q(decrease verbosity)\n\
+  c(continue), s(silent).\n\
+Verbosity (default 1) controls what to print for a thread:\n\
+  0 : no thread info is printed\n\
+  1 : print per-inferior thread number and target system's thread id\n\
+By default, if a COMMAND raises an error, thread apply is aborted.\n\
+Flag c indicates to print the error and continue.\n\
+Flag s indicates to silently ignore a COMMAND that raises an error\n\
+or produces no output."
+
   add_prefix_cmd ("apply", class_run, thread_apply_command,
 		  _("Apply a command to a list of threads.\n\
-Usage: thread apply ID... COMMAND\n\
-ID is a space-separated list of IDs of threads to apply COMMAND on."),
+Usage: thread apply ID... [-FLAGS...] COMMAND\n\
+ID is a space-separated list of IDs of threads to apply COMMAND on.\n"
+THREAD_APPLY_FLAGS_HELP),
 		  &thread_apply_list, "thread apply ", 1, &thread_cmd_list);
 
   add_cmd ("all", class_run, thread_apply_all_command,
 	   _("\
 Apply a command to all threads.\n\
 \n\
-Usage: thread apply all [-ascending] COMMAND\n\
+Usage: thread apply all [-ascending] [-FLAGS...] COMMAND\n\
 -ascending: Call COMMAND for all threads in ascending order.\n\
-            The default is descending order.\
-"),
+            The default is descending order.\n"
+THREAD_APPLY_FLAGS_HELP),
 	   &thread_apply_list);
 
+  add_com ("taas", class_run, taas_command, _("\
+Apply a command to all threads (ignoring errors and empty output)\n\
+Usage: taas COMMAND\n\
+shortcut for 'thread apply all -s COMMAND'"));
+
+  add_com ("tfaas", class_run, tfaas_command, _("\
+Apply a command to all frames of all threads (ignoring errors and empty output)\n\
+Usage: tfaas COMMAND\n\
+shortcut for 'thread apply all -s frame apply all -s COMMAND'"));
+
   add_cmd ("name", class_run, thread_name_command,
 	   _("Set the current thread's name.\n\
 Usage: thread name [NAME]\n\
-- 
2.11.0

Re: [RFC 3/5] Add -FLAGS... argument to thread apply

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  5 May 2018 21:28:02 +0200
> 
>    if (cmd == NULL || *cmd == '\000')
> -    error (_("Please specify a command following the thread ID list"));
> +    error (_("Please specify a command at the end of thread apply all"));

I think "thread apply all" should be in quotes here.

> +  add_com ("taas", class_run, taas_command, _("\
> +Apply a command to all threads (ignoring errors and empty output)\n\

The first line of a doc string should end with a period, as some help
commands rely on that.

> +  add_com ("tfaas", class_run, tfaas_command, _("\
> +Apply a command to all frames of all threads (ignoring errors and empty output)\n\

Ditto.

Thanks.

[RFC 5/5] Announce 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for frame apply

[Philippe Waroquiers]

Announce the user visible changes.
---
 gdb/NEWS | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index cef558039e..8ee4db985d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -14,6 +14,24 @@
 
 * New commands
 
+frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
+  Apply a command to a number of frames.
+  The FLAGS allows to control what output to produce and how to handle
+  errors raised when applying COMMAND to a frame.
+
+taas COMMAND
+  Apply a command to all threads (ignoring errors and empty output)
+  Shortcut for 'thread apply all -s COMMAND'
+
+faas COMMAND
+  Apply a command to all frames (ignoring errors and empty output)
+  Shortcut for 'frame apply all -s COMMAND'
+
+tfaas COMMAND
+  Apply a command to all frames of all threads (ignoring errors and empty
+  output).
+  Shortcut for 'thread apply all -s frame apply all -s COMMAND'
+
 set debug fbsd-nat
 show debug fbsd-nat
   Control display of debugging info regarding the FreeBSD native target.
@@ -27,6 +45,13 @@ set|show record btrace cpu
   Controls the processor to be used for enabling errata workarounds for
   branch trace decode.
 
+* Changed commands
+
+thread apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
+  The thread apply command accepts a new argument FLAGS.
+  The FLAGS allows to control what output to produce and how to handle errors
+  raised when applying COMMAND to a frame.
+
 * Python API
 
   ** Type alignment is now exposed via the "align" attribute of a gdb.Type.
-- 
2.11.0

Re: [RFC 5/5] Announce 'frame apply', faas, taas, tfaas commands and -FLAGS... arg for frame apply

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat,  5 May 2018 21:28:04 +0200
> 
> Announce the user visible changes.
> ---
>  gdb/NEWS | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index cef558039e..8ee4db985d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -14,6 +14,24 @@
>  
>  * New commands
>  
> +frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
> +  Apply a command to a number of frames.
> +  The FLAGS allows to control what output to produce and how to handle
> +  errors raised when applying COMMAND to a frame.
> +
> +taas COMMAND
> +  Apply a command to all threads (ignoring errors and empty output)

Please add a period at the end of this sentence.

> +  Shortcut for 'thread apply all -s COMMAND'

Likewise.

> +faas COMMAND
> +  Apply a command to all frames (ignoring errors and empty output)

Likewise,

> +  Shortcut for 'frame apply all -s COMMAND'

Likewise.

> +
> +tfaas COMMAND
> +  Apply a command to all frames of all threads (ignoring errors and empty
> +  output).
> +  Shortcut for 'thread apply all -s frame apply all -s COMMAND'

Likewise.

> +
>  set debug fbsd-nat
>  show debug fbsd-nat
>    Control display of debugging info regarding the FreeBSD native target.
> @@ -27,6 +45,13 @@ set|show record btrace cpu
>    Controls the processor to be used for enabling errata workarounds for
>    branch trace decode.
>  
> +* Changed commands
> +
> +thread apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND
> +  The thread apply command accepts a new argument FLAGS.

Please quote "thread apply" 'like this'.

Thanks.

Re: [RFC 0/5] Implenent 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Simon Marchi]

On 2018-05-05 03:27 PM, Philippe Waroquiers wrote:
> This patch series :
>  * implements a new command
>      'frame apply [all | COUNT | -COUNT] [-FLAGS...] COMMAND'.
>  * enhance 'thread apply COMMAND' by adding a -FLAGS argument
>  * adds some shortcuts commands
>  * documents the above in gdb.texinfo and NEWS.
> 
> Th new command 'frame apply' allows to apply a COMMAND to a number of frames,
> or to all frames.
> The optional -FLAGS... argument allows to control what output to produce
> and how to handle errors raised when applying COMMAND to a frame.
> 
> Some examples usages for this new command:
>    frame apply all info frame
>       Produce info frame for all frames
>    frame apply all p $sp
>       For each frame, print the location, followed by the frame sp
>    frame apply all -qq p $sp
>       Same as before, but -qq flags (q = quiet) indicate to only print
>       the frames sp.
>    frame apply all -vv p $sp
>       Same as before, but -vv flags (v = verbose) indicate to print
>       location and source line for each frame.
>    frame apply all p some_local_var_somewhere
>       Print some_local_var_somewhere in all frames. 'frame apply'
>       will abort as soon as the print command fails.
>    frame apply all -c p some_local_var_somewhere
>       Same as before, but -c flag (c = continue) means to
>       print the error and continue applying command in case the
>       print command fails.
>    frame apply all -s p some_local_var_somewhere
>       Same as before, but -s flag (s = silent) means to
>       be silent for frames where the print command fails.
>       In other words, this allows to 'search' the frame in which
>       some_local_var_somewhere can be printed.
> 
> 'thread apply' command has been enhanced to also accepts a -FLAGS...
> argument.
> 
> Some examples usages for this new argument:
>    thread apply all -s frame apply all -s p some_local_var_somewhere
>       Prints the thread id, frame location and some_local_var_somewhere
>       value in frames of threads that have such local var.
> 
> To make the life of the user easier, the most typical use cases
> have shortcuts :
>    faas  : shortcut for 'frame apply all -s'
>    taas  : shortcut for 'thread apply all -s'
>    tfaas : shortcut for 'thread apply all -s frame apply all -s"
> 
> An example usage :
>    tfaas p some_local_var_somewhere
>      same as the longer:
>         'thread apply all -s frame apply all -s p some_local_var_somewhere'
> 
> The patch serie provides the implementation and all user visible documentation.
> 
> What is still missing is:
>    * Missing regression tests.
>      Not yet done as this is painful, and so is better done once there is
>      an agreement on the behaviour.
> 
>    * Missing ChangeLog.
>      Not yet done as this is painful, and is IMO close to or completely useless.
>      So, I will do it for the (first) RFA, once the flow of RFC comments
>      stops :).
> 

Hi Philippe,

I think this is really nice.  I was really amazed by "thread apply all frame apply all",
simple but powerful.

As noted in my other message, I am not sure if it's a good idea to have similar syntax
but different semantic for "thread apply" and "frame apply", it might confuse more
than anything.  I'm curious to know what you think about it.  Other than that, I don't
have any strong opinion on the syntax, it seems fine to me.

Simon

Re: [RFC 0/5] Implenent 'frame apply COMMAND', enhance 'thread apply COMMAND'

[Philippe Waroquiers]

On Thu, 2018-05-17 at 22:14 -0400, Simon Marchi wrote:
> Hi Philippe,
> 
> I think this is really nice.  I was really amazed by "thread apply all frame apply all",
> simple but powerful.
> 
> As noted in my other message, I am not sure if it's a good idea to have similar syntax
> but different semantic for "thread apply" and "frame apply", it might confuse more
> than anything.  I'm curious to know what you think about it.  Other than that, I don't
> have any strong opinion on the syntax, it seems fine to me.
> 
> Simon
Hello Simon,

There are a few comments to which I will reply specifically
in the various mails.
The other non discussed comments are all clear to me, and
I should be able to handle them for the next version.

Thanks for the review.

Philippe