[RFAv2 0/6] Implement | (pipe) command.

[RFAv2 0/6] Implement | (pipe) command.

[Philippe Waroquiers]

Implement | (pipe) command.

This patch series adds the pipe command, that allows to send the output
of a GDB command to a shell command.

This version handles the comments received.
As there is a new option -dX to the pipe_command, the doc and help
was changed to describe it.

* Comments from Eli :
   - better definition of WIF* macros for MinGW.
   - replace @ref by @xref in the doc.

* Comment from Abhijit Halder/Tom:
   it not that unlikely to have | in a GDB command
   => an optional -dX option allows to specify an
   alternate character to use X to replace the | as separator
   between the GDB COMMAND and the SHELL_COMMAND.

* Comments from Tom:
  * make previous_saved_command_line static. For this, saved_command_line
    is now also static, and all repeat related functions/vars are now
    in top.c
  * various small changes (use std::swap, strchr, .empty (), ...).
  * removed the scoped_restore_current_thread restore
  * popen has been kept as libiberty pexecute still implies to use
    the WIF* macros.
  * Instead of using execute_command_to_string, use GDB redirection
    mechanism.  I did several trials for this, and at the end,
    the only one working properly was very close to the code
    of execute_command_to_string.
    => we now have a function execute_command_to_ui_file that is used
    to implement the pipe command, and also used by
    execute_command_to_string.

[RFAv2 3/6] Add function execute_command_to_ui_file

[Philippe Waroquiers]

2019-04-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdbcmd.h (execute_command_to_ui_file): New declaration.
	top.c (execute_command_to_ui_file): New function, mostly a copy
	of execute_command_to_string.
	(execute_command_to_string): Implement by calling
	execute_command_to_ui_file.
---
 gdb/gdbcmd.h |  2 ++
 gdb/top.c    | 33 +++++++++++++++++++++------------
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/gdb/gdbcmd.h b/gdb/gdbcmd.h
index 4614ec748c..14c0b80bac 100644
--- a/gdb/gdbcmd.h
+++ b/gdb/gdbcmd.h
@@ -134,6 +134,8 @@ extern struct cmd_list_element *save_cmdlist;
 
 extern void execute_command (const char *, int);
 extern std::string execute_command_to_string (const char *p, int from_tty);
+extern void execute_command_to_ui_file (struct ui_file *file,
+					const char *p, int from_tty);
 
 extern void print_command_line (struct command_line *, unsigned int,
 				struct ui_file *);
diff --git a/gdb/top.c b/gdb/top.c
index 93fed2ed80..bd68ba182b 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -665,12 +665,12 @@ execute_command (const char *p, int from_tty)
   cleanup_if_error.release ();
 }
 
-/* Run execute_command for P and FROM_TTY.  Capture its output into the
-   returned string, do not display it to the screen.  BATCH_FLAG will be
+/* Run execute_command for P and FROM_TTY.  Sends its output to FILE,
+   do not display it to the screen.  BATCH_FLAG will be
    temporarily set to true.  */
 
-std::string
-execute_command_to_string (const char *p, int from_tty)
+void
+execute_command_to_ui_file (struct ui_file *file, const char *p, int from_tty)
 {
   /* GDB_STDOUT should be better already restored during these
      restoration callbacks.  */
@@ -678,26 +678,35 @@ execute_command_to_string (const char *p, int from_tty)
 
   scoped_restore save_async = make_scoped_restore (&current_ui->async, 0);
 
-  string_file str_file;
-
   {
-    current_uiout->redirect (&str_file);
+    current_uiout->redirect (file);
     ui_out_redirect_pop redirect_popper (current_uiout);
 
     scoped_restore save_stdout
-      = make_scoped_restore (&gdb_stdout, &str_file);
+      = make_scoped_restore (&gdb_stdout, file);
     scoped_restore save_stderr
-      = make_scoped_restore (&gdb_stderr, &str_file);
+      = make_scoped_restore (&gdb_stderr, file);
     scoped_restore save_stdlog
-      = make_scoped_restore (&gdb_stdlog, &str_file);
+      = make_scoped_restore (&gdb_stdlog, file);
     scoped_restore save_stdtarg
-      = make_scoped_restore (&gdb_stdtarg, &str_file);
+      = make_scoped_restore (&gdb_stdtarg, file);
     scoped_restore save_stdtargerr
-      = make_scoped_restore (&gdb_stdtargerr, &str_file);
+      = make_scoped_restore (&gdb_stdtargerr, file);
 
     execute_command (p, from_tty);
   }
+}
+
+/* Run execute_command for P and FROM_TTY.  Capture its output into the
+   returned string, do not display it to the screen.  BATCH_FLAG will be
+   temporarily set to true.  */
+
+std::string
+execute_command_to_string (const char *p, int from_tty)
+{
+  string_file str_file;
 
+  execute_command_to_ui_file (&str_file, p, from_tty);
   return std::move (str_file.string ());
 }
 
-- 
2.20.1

[RFAv2 4/6] Implement | (pipe) command.

[Philippe Waroquiers]

The pipe command allows to run a GDB command, and pipe its output
to a shell command:
  (gdb) help pipe
  Send the output of a gdb command to a shell command.
  Usage: pipe [COMMAND] | SHELL_COMMAND
  Usage: | [COMMAND] | SHELL_COMMAND
  Usage: pipe -dX COMMAND X SHELL_COMMAND
  Usage: | -dX COMMAND X SHELL_COMMAND
  Executes COMMAND and sends its output to SHELL_COMMAND.
  If COMMAND contains a | character, the option -dX indicates
  to use the character X to separate COMMAND from SHELL_COMMAND.
  With no COMMAND, repeat the last executed command
  and send its output to SHELL_COMMAND.
  (gdb)

For example:
  (gdb) pipe print some_data_structure | grep -B3 -A3 something

The pipe character is defined as an alias for pipe command, so that
the above can be typed as:
  (gdb) | print some_data_structure | grep -B3 -A3 something

If no GDB COMMAND is given, then the previous command is relaunched,
and its output is sent to the given SHELL_COMMAND.

gdb/ChangeLog
2019-04-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* cli/cli-cmds.c (pipe_command): New function.
	(_initialize_cli_cmds): Call add_com for pipe_command.
	Define | as an alias for pipe.
	cli/cli-decode.c (find_command_name_length): Recognize | as
	a single character command.
---
 gdb/cli/cli-cmds.c   | 110 +++++++++++++++++++++++++++++++++++++++++++
 gdb/cli/cli-decode.c |   4 +-
 2 files changed, 112 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5f3b973f06..eefdc2cd02 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -24,6 +24,7 @@
 #include "completer.h"
 #include "target.h"	/* For baud_rate, remote_debug and remote_timeout.  */
 #include "common/gdb_wait.h"	/* For shell escape implementation.  */
+#include "gdbcmd.h"
 #include "gdb_regex.h"	/* Used by apropos_command.  */
 #include "gdb_vfork.h"
 #include "linespec.h"
@@ -41,6 +42,7 @@
 #include "block.h"
 
 #include "ui-out.h"
+#include "interps.h"
 
 #include "top.h"
 #include "cli/cli-decode.h"
@@ -854,6 +856,101 @@ edit_command (const char *arg, int from_tty)
   xfree (p);
 }
 
+/* Implementation of the "pipe" command.  */
+
+static void
+pipe_command (const char *arg, int from_tty)
+{
+  const char *command = arg;
+  const char *shell_command = arg;
+  int separator = '|';
+
+  if (arg == NULL)
+    error (_("Missing \"[COMMAND] | SHELL_COMMAND\""));
+
+  shell_command = skip_spaces (shell_command);
+
+  if (*shell_command == '-' && *(shell_command + 1) == 'd')
+    {
+      separator = *(shell_command + 2);
+      if (separator == 0 || separator == ' ')
+	error (_("Missing separator X after -d in "
+		 "\"| -dX COMMAND X SHELL_COMMAND\""));
+      shell_command += 3;
+      command = shell_command;
+    }
+
+  shell_command = strchr (shell_command, separator);
+
+  if (shell_command == nullptr)
+    {
+      if (separator == '|')
+	error (_("Missing | SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
+      else
+	error (_("Missing %c SHELL_COMMAND in "
+		 "\"| -d%c COMMAND %c SHELL_COMMAND\""),
+	       separator, separator, separator);
+    }
+
+  command = skip_spaces (command);
+  std::string gdb_cmd (command, shell_command - command);
+
+  if (gdb_cmd.empty ())
+    {
+      repeat_previous ();
+      gdb_cmd = skip_spaces (get_saved_command_line ());
+      if (gdb_cmd.empty ())
+	error (_("No previous command to relaunch"));
+    }
+
+  shell_command++; /* skip the separator.  */
+  shell_command = skip_spaces (shell_command);
+  if (*shell_command == 0)
+    {
+      if (separator == '|')
+	error (_("Missing SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
+      else
+	error (_("Missing SHELL_COMMAND in "
+		 "\"| -d%c COMMAND %c SHELL_COMMAND\""),
+	       separator, separator);
+    }
+
+  FILE *to_shell_command = popen (shell_command, "w");
+
+  if (to_shell_command == NULL)
+    error (_("Error launching \"%s\""), shell_command);
+
+  try
+    {
+      stdio_file pipe_file (to_shell_command);
+
+      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      pclose (to_shell_command);
+      throw;
+    }
+
+  int shell_command_status = pclose (to_shell_command);
+
+  if (shell_command_status < 0)
+    error (_("shell command  \"%s\" errno %s"), shell_command,
+           safe_strerror (errno));
+  if (shell_command_status > 0)
+    {
+      if (WIFEXITED (shell_command_status))
+	warning (_("shell command \"%s\" exit status %d"), shell_command,
+		 WEXITSTATUS (shell_command_status));
+      else if (WIFSIGNALED (shell_command_status))
+	warning (_("shell command \"%s\" exit with signal %d"), shell_command,
+		 WTERMSIG (shell_command_status));
+      else
+	warning (_("shell command \"%s\" status %d"), shell_command,
+		 shell_command_status);
+    }
+}
+
 static void
 list_command (const char *arg, int from_tty)
 {
@@ -1845,6 +1942,19 @@ Uses EDITOR environment variable contents as editor (or ex as default)."));
 
   c->completer = location_completer;
 
+  c = add_com ("pipe", class_support, pipe_command, _("\
+Send the output of a gdb command to a shell command.\n\
+Usage: pipe [COMMAND] | SHELL_COMMAND\n\
+Usage: | [COMMAND] | SHELL_COMMAND\n\
+Usage: pipe -dX COMMAND X SHELL_COMMAND\n\
+Usage: | -dX COMMAND X SHELL_COMMAND\n\
+Executes COMMAND and sends its output to SHELL_COMMAND.\n\
+If COMMAND contains a | character, the option -dX indicates\n\
+to use the character X to separate COMMAND from SHELL_COMMAND.\n\
+With no COMMAND, repeat the last executed command\n\
+and send its output to SHELL_COMMAND."));
+  add_com_alias ("|", "pipe", class_support, 0);
+
   add_com ("list", class_files, list_command, _("\
 List specified function or line.\n\
 With no argument, lists ten more lines after or around previous listing.\n\
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 50430953c7..46051090cd 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1311,9 +1311,9 @@ find_command_name_length (const char *text)
      Note that this is larger than the character set allowed when
      creating user-defined commands.  */
 
-  /* Recognize '!' as a single character command so that, e.g., "!ls"
+  /* Recognize the single character commands so that, e.g., "!ls"
      works as expected.  */
-  if (*p == '!')
+  if (*p == '!' || *p == '|')
     return 1;
 
   while (isalnum (*p) || *p == '-' || *p == '_'
-- 
2.20.1

Re: [RFAv2 4/6] Implement | (pipe) command.

[Pedro Alves]

On 4/26/19 9:11 PM, Philippe Waroquiers wrote:
> The pipe command allows to run a GDB command, and pipe its output
> to a shell command:
>   (gdb) help pipe
>   Send the output of a gdb command to a shell command.
>   Usage: pipe [COMMAND] | SHELL_COMMAND
>   Usage: | [COMMAND] | SHELL_COMMAND
>   Usage: pipe -dX COMMAND X SHELL_COMMAND
>   Usage: | -dX COMMAND X SHELL_COMMAND
>   Executes COMMAND and sends its output to SHELL_COMMAND.
>   If COMMAND contains a | character, the option -dX indicates
>   to use the character X to separate COMMAND from SHELL_COMMAND.
>   With no COMMAND, repeat the last executed command
>   and send its output to SHELL_COMMAND.
>   (gdb)

I think that making "-dX" a single character is a mistake.  I'd rather
make that "-d STRING", so you can use use a string that is much less
likely to conflict.  Like bash herestrings.  So a user would be able
to do:

pipe -d XXXYYYXXX $command | $shell_command

I also think "-dX" without a space in between is very non-gdb-ish.
In gdb, you kind of either have "-OPT X", or "/OX", which is kind of
double "--" vs "-" with getopt, except with different leading
tokens.

> 
> For example:
>   (gdb) pipe print some_data_structure | grep -B3 -A3 something
> 
> The pipe character is defined as an alias for pipe command, so that
> the above can be typed as:
>   (gdb) | print some_data_structure | grep -B3 -A3 something
> 

I get that "it makes sense", but do you see yourself using the "|" command
in preference to "pipe"?  "pipe" can be typed as "pi", which is two
key strokes as well.  Kind of wondering whether the character could be
saved for something else in the future.  I don't have a use in mind,
just thinking out loud.

> If no GDB COMMAND is given, then the previous command is relaunched,
> and its output is sent to the given SHELL_COMMAND.
> 

> +  command = skip_spaces (command);
> +  std::string gdb_cmd (command, shell_command - command);
> +
> +  if (gdb_cmd.empty ())
> +    {
> +      repeat_previous ();
> +      gdb_cmd = skip_spaces (get_saved_command_line ());
> +      if (gdb_cmd.empty ())
> +	error (_("No previous command to relaunch"));
> +    }
> +
> +  shell_command++; /* skip the separator.  */

Uppercase "Skip".

> +  shell_command = skip_spaces (shell_command);
> +  if (*shell_command == 0)

  if (*shell_command == '\0')


> +    {
> +      if (separator == '|')

I don't think this check is 100% right, considering that you could do:

(gdb) pipe -d| SHELL_COMMAND

I.e., what you want to check is whether a separate was explicitly specified.

> +	error (_("Missing SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
> +      else
> +	error (_("Missing SHELL_COMMAND in "
> +		 "\"| -d%c COMMAND %c SHELL_COMMAND\""),
> +	       separator, separator);

Or instead of making this dynamic, just print something like:

	error (_("Missing SHELL_COMMAND.\n"
                 "Usage: \"| [-c SEP] [COMMAND] | SHELL_COMMAND\""));


> +    }
> +
> +  FILE *to_shell_command = popen (shell_command, "w");
> +
> +  if (to_shell_command == NULL)
> +    error (_("Error launching \"%s\""), shell_command);
> +
> +  try
> +    {
> +      stdio_file pipe_file (to_shell_command);
> +
> +      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);
> +    }
> +  catch (const gdb_exception_error &ex)
> +    {
> +      pclose (to_shell_command);
> +      throw;
> +    }
> +
> +  int shell_command_status = pclose (to_shell_command);
> +
> +  if (shell_command_status < 0)
> +    error (_("shell command  \"%s\" errno %s"), shell_command,
> +           safe_strerror (errno));
> +  if (shell_command_status > 0)
> +    {
> +      if (WIFEXITED (shell_command_status))
> +	warning (_("shell command \"%s\" exit status %d"), shell_command,
> +		 WEXITSTATUS (shell_command_status));
> +      else if (WIFSIGNALED (shell_command_status))
> +	warning (_("shell command \"%s\" exit with signal %d"), shell_command,
> +		 WTERMSIG (shell_command_status));
> +      else
> +	warning (_("shell command \"%s\" status %d"), shell_command,
> +		 shell_command_status);
> +    }

Is there a reason this isn't using the pex routines?

Thanks,
Pedro Alves

Re: [RFAv2 4/6] Implement | (pipe) command.

[Philippe Waroquiers]

On Fri, 2019-05-03 at 19:59 +0100, Pedro Alves wrote:
> On 4/26/19 9:11 PM, Philippe Waroquiers wrote:
> > The pipe command allows to run a GDB command, and pipe its output
> > to a shell command:
> >   (gdb) help pipe
> >   Send the output of a gdb command to a shell command.
> >   Usage: pipe [COMMAND] | SHELL_COMMAND
> >   Usage: | [COMMAND] | SHELL_COMMAND
> >   Usage: pipe -dX COMMAND X SHELL_COMMAND
> >   Usage: | -dX COMMAND X SHELL_COMMAND
> >   Executes COMMAND and sends its output to SHELL_COMMAND.
> >   If COMMAND contains a | character, the option -dX indicates
> >   to use the character X to separate COMMAND from SHELL_COMMAND.
> >   With no COMMAND, repeat the last executed command
> >   and send its output to SHELL_COMMAND.
> >   (gdb)
> 
> I think that making "-dX" a single character is a mistake.  I'd rather
> make that "-d STRING", so you can use use a string that is much less
> likely to conflict.  Like bash herestrings.  So a user would be able
> to do:
> 
> pipe -d XXXYYYXXX $command | $shell_command
> 
> I also think "-dX" without a space in between is very non-gdb-ish.
> In gdb, you kind of either have "-OPT X", or "/OX", which is kind of
> double "--" vs "-" with getopt, except with different leading
> tokens.

Ok, will change to -d STRING.


> 
> > 
> > For example:
> >   (gdb) pipe print some_data_structure | grep -B3 -A3 something
> > 
> > The pipe character is defined as an alias for pipe command, so that
> > the above can be typed as:
> >   (gdb) | print some_data_structure | grep -B3 -A3 something
> > 
> 
> I get that "it makes sense", but do you see yourself using the "|" command
> in preference to "pipe"?  "pipe" can be typed as "pi", which is two
> key strokes as well.  Kind of wondering whether the character could be
> saved for something else in the future.  I don't have a use in mind,
> just thinking out loud.
Yes, I always use "|" (I even just checked the test to be sure that I
also tested "pipe" and not only "|").
pip must be used, as pi launches an interactive python.

Also, "|" allows to do:
(gdb) some_command
...
(gdb) ||grep something_in_some_command_output
(gdb) ||tee save_some_command_output

which is better than
(gdb) pip|grep something_in_some_command_output

> 
> > If no GDB COMMAND is given, then the previous command is relaunched,
> > and its output is sent to the given SHELL_COMMAND.
> > 
> > +  command = skip_spaces (command);
> > +  std::string gdb_cmd (command, shell_command - command);
> > +
> > +  if (gdb_cmd.empty ())
> > +    {
> > +      repeat_previous ();
> > +      gdb_cmd = skip_spaces (get_saved_command_line ());
> > +      if (gdb_cmd.empty ())
> > +	error (_("No previous command to relaunch"));
> > +    }
> > +
> > +  shell_command++; /* skip the separator.  */
> 
> Uppercase "Skip".
> 
> > +  shell_command = skip_spaces (shell_command);
> > +  if (*shell_command == 0)
> 
>   if (*shell_command == '\0')
> 
> 
> > +    {
> > +      if (separator == '|')
> 
> I don't think this check is 100% right, considering that you could do:
That part of the code will change heavily when the separator becomes a string.
> 
> (gdb) pipe -d| SHELL_COMMAND
> 
> I.e., what you want to check is whether a separate was explicitly specified.
> 
> > +	error (_("Missing SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
> > +      else
> > +	error (_("Missing SHELL_COMMAND in "
> > +		 "\"| -d%c COMMAND %c SHELL_COMMAND\""),
> > +	       separator, separator);
> 
> Or instead of making this dynamic, just print something like:
> 
> 	error (_("Missing SHELL_COMMAND.\n"
>                  "Usage: \"| [-c SEP] [COMMAND] | SHELL_COMMAND\""));

> 
> 
> > +    }
> > +
> > +  FILE *to_shell_command = popen (shell_command, "w");
> > +
> > +  if (to_shell_command == NULL)
> > +    error (_("Error launching \"%s\""), shell_command);
> > +
> > +  try
> > +    {
> > +      stdio_file pipe_file (to_shell_command);
> > +
> > +      execute_command_to_ui_file (&pipe_file, gdb_cmd.c_str (), from_tty);
> > +    }
> > +  catch (const gdb_exception_error &ex)
> > +    {
> > +      pclose (to_shell_command);
> > +      throw;
> > +    }
> > +
> > +  int shell_command_status = pclose (to_shell_command);
> > +
> > +  if (shell_command_status < 0)
> > +    error (_("shell command  \"%s\" errno %s"), shell_command,
> > +           safe_strerror (errno));
> > +  if (shell_command_status > 0)
> > +    {
> > +      if (WIFEXITED (shell_command_status))
> > +	warning (_("shell command \"%s\" exit status %d"), shell_command,
> > +		 WEXITSTATUS (shell_command_status));
> > +      else if (WIFSIGNALED (shell_command_status))
> > +	warning (_("shell command \"%s\" exit with signal %d"), shell_command,
> > +		 WTERMSIG (shell_command_status));
> > +      else
> > +	warning (_("shell command \"%s\" status %d"), shell_command,
> > +		 shell_command_status);
> > +    }
> 
> Is there a reason this isn't using the pex routines?
pex routines do not help to handle the exit status (and are
also not used for "shell").

However your question now makes me believe that
it is not a good idea to systematically report to the user
the exit status of the launched command.
E.g. in a shell, if you do:
   $ echo something | grep somethingelse
the shell does not tell you:
  warning: grep has exited with status 1
which is what the GDB patch does now !
I think it is up to each command to have its way to 
report success or failure.

So, I believe the correct solution is to have a convenience variable
(e.g. $_exit_status) set with the exit status of the last launched
command (either by the GDB shell or pipe command).
This convenience variable can then be looked at by the user if there
is a doubt about how the last command worked.
And more interesting, this convenience variable can be used
in user defined commands.

I will prepare a RFAv3 based on this.

Thanks for the review

Philippe

Re: [RFAv2 4/6] Implement | (pipe) command.

[Pedro Alves]

On 5/4/19 7:24 AM, Philippe Waroquiers wrote:
> On Fri, 2019-05-03 at 19:59 +0100, Pedro Alves wrote:
>> On 4/26/19 9:11 PM, Philippe Waroquiers wrote:
>>> The pipe command allows to run a GDB command, and pipe its output
>>> to a shell command:
>>>   (gdb) help pipe
>>>   Send the output of a gdb command to a shell command.
>>>   Usage: pipe [COMMAND] | SHELL_COMMAND
>>>   Usage: | [COMMAND] | SHELL_COMMAND
>>>   Usage: pipe -dX COMMAND X SHELL_COMMAND
>>>   Usage: | -dX COMMAND X SHELL_COMMAND
>>>   Executes COMMAND and sends its output to SHELL_COMMAND.
>>>   If COMMAND contains a | character, the option -dX indicates
>>>   to use the character X to separate COMMAND from SHELL_COMMAND.
>>>   With no COMMAND, repeat the last executed command
>>>   and send its output to SHELL_COMMAND.
>>>   (gdb)
>>
>> I think that making "-dX" a single character is a mistake.  I'd rather
>> make that "-d STRING", so you can use use a string that is much less
>> likely to conflict.  Like bash herestrings.  So a user would be able
>> to do:
>>
>> pipe -d XXXYYYXXX $command | $shell_command
>>
>> I also think "-dX" without a space in between is very non-gdb-ish.
>> In gdb, you kind of either have "-OPT X", or "/OX", which is kind of
>> double "--" vs "-" with getopt, except with different leading
>> tokens.
> 
> Ok, will change to -d STRING.
> 

Thanks.

>>> For example:
>>>   (gdb) pipe print some_data_structure | grep -B3 -A3 something
>>>
>>> The pipe character is defined as an alias for pipe command, so that
>>> the above can be typed as:
>>>   (gdb) | print some_data_structure | grep -B3 -A3 something
>>>
>>
>> I get that "it makes sense", but do you see yourself using the "|" command
>> in preference to "pipe"?  "pipe" can be typed as "pi", which is two
>> key strokes as well.  Kind of wondering whether the character could be
>> saved for something else in the future.  I don't have a use in mind,
>> just thinking out loud.
> Yes, I always use "|" (I even just checked the test to be sure that I
> also tested "pipe" and not only "|").
> pip must be used, as pi launches an interactive python.
> 
> Also, "|" allows to do:
> (gdb) some_command
> ...
> (gdb) ||grep something_in_some_command_output
> (gdb) ||tee save_some_command_output
> 
> which is better than
> (gdb) pip|grep something_in_some_command_output

Alright, that does sound clever.

Thanks,
Pedro Alves

[RFAv2 2/6] Improve process exit status macros on MinGW

[Philippe Waroquiers]

gdb/ChangeLog
2019-04-26  Eli Zaretskii  <eliz@gnu.org>
	    Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* common/gdb_wait.h (WIFEXITED, WIFSIGNALED, WEXITSTATUS,
	WTERMSIG): Define better versions for MinGW.
	* windows-nat.c (xlate): Uncomment the definition.
	(windows_status_to_termsig): New function.
---
 gdb/common/gdb_wait.h | 27 +++++++++++++++++++++++++++
 gdb/windows-nat.c     | 18 ++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/gdb/common/gdb_wait.h b/gdb/common/gdb_wait.h
index b3b752cf3a..ca95240009 100644
--- a/gdb/common/gdb_wait.h
+++ b/gdb/common/gdb_wait.h
@@ -40,13 +40,31 @@
    NOTE exception for GNU/Linux below).  We also fail to declare
    wait() and waitpid().  */
 
+/* For MINGW, the underlying idea is that when a Windows program is terminated
+   by a fatal exception, its exit code is the value of that exception, as
+   defined by the various STATUS_* symbols in the Windows API headers.
+
+   The below is not perfect, because a program could legitimately exit normally
+   with a status whose value happens to have the high bits set, but that's
+   extremely rare, to say the least, and it is deemed such a negligibly small
+   probability of false positives is justified by the utility of reporting the
+   terminating signal in the "normal" cases.  */
+
 #ifndef	WIFEXITED
+#if defined (__MINGW32__)
+#define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
+#else
 #define WIFEXITED(w)	(((w)&0377) == 0)
 #endif
+#endif
 
 #ifndef	WIFSIGNALED
+#if defined (__MINGW32__)
+#define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) == 0xC0000000)
+#else
 #define WIFSIGNALED(w)	(((w)&0377) != 0177 && ((w)&~0377) == 0)
 #endif
+#endif
 
 #ifndef	WIFSTOPPED
 #ifdef IBM6000
@@ -64,12 +82,21 @@
 #endif
 
 #ifndef	WEXITSTATUS
+#if defined (__MINGW32__)
+#define WEXITSTATUS(stat_val) ((stat_val) & 255)
+#else
 #define WEXITSTATUS(w)	(((w) >> 8) & 0377) /* same as WRETCODE */
 #endif
+#endif
 
 #ifndef	WTERMSIG
+#if defined (__MINGW32__)
+extern enum gdb_signal windows_status_to_termsig (int stat_val);
+#define WTERMSIG(stat_val)    windows_status_to_termsig (stat_val)
+#else
 #define WTERMSIG(w)	((w) & 0177)
 #endif
+#endif
 
 #ifndef	WSTOPSIG
 #define WSTOPSIG	WEXITSTATUS
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 50094187bd..411c7a8133 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -281,9 +281,9 @@ static const int *mappings;
    a segment register or not.  */
 static segment_register_p_ftype *segment_register_p;
 
-/* See windows_nat_target::resume to understand why this is commented
-   out.  */
-#if 0
+/* See windows_nat_target::resume to understand why xlate is not used
+   to translate a signal into an exception.  */
+
 /* This vector maps the target's idea of an exception (extracted
    from the DEBUG_EVENT structure) to GDB's idea.  */
 
@@ -303,7 +303,17 @@ static const struct xlate_exception xlate[] =
   {STATUS_FLOAT_DIVIDE_BY_ZERO, GDB_SIGNAL_FPE}
 };
 
-#endif /* 0 */
+/* Translate a windows exception inside STAT_VAL into a gdb_signal.
+   This should only be called if WIFSIGNALED (stat_val).  */
+
+enum gdb_signal
+windows_status_to_termsig (int stat_val)
+{
+  for (const xlate_exception &x : xlate)
+    if (x.them == (stat_val & ~0xC0000000))
+      return x.us;
+  return GDB_SIGNAL_UNKNOWN;
+}
 
 struct windows_nat_target final : public x86_nat_target<inf_child_target>
 {
-- 
2.20.1

[RFAv2 6/6] NEWS and documentation for | (pipe) command.

[Philippe Waroquiers]

gdb/ChangeLog
	* NEWS: Mention new pipe command.

gdb/doc/ChangeLog
	* gdb.texinfo (Shell Commands): Document pipe command.
	(Logging Output): Add a reference to pipe command.
---
 gdb/NEWS            |  8 ++++++++
 gdb/doc/gdb.texinfo | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 5309a8f923..8744659e7b 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -27,6 +27,14 @@
      'array_indexes', 'symbols', 'unions', 'deref_refs', 'actual_objects',
      'static_members', 'max_elements', 'repeat_threshold', and 'format'.
 
+* New commands
+
+pipe [COMMAND] | SHELL_COMMAND
+| [COMMAND] | SHELL_COMMAND
+  Executes COMMAND and sends its output to SHELL_COMMAND.
+  With no COMMAND, repeat the last executed command
+  and send its output to SHELL_COMMAND.
+
 *** Changes in GDB 8.3
 
 * GDB and GDBserver now support access to additional registers on
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 0733e1acfd..9467b8c4ec 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1454,6 +1454,43 @@ Execute the @code{make} program with the specified
 arguments.  This is equivalent to @samp{shell make @var{make-args}}.
 @end table
 
+@table @code
+@kindex pipe
+@kindex |
+@cindex send the output of a gdb command to a shell command
+@anchor{pipe}
+@item pipe [@var{command}] | @var{shell_command}
+@itemx | [@var{command}] | @var{shell_command}
+@item pipe -d@var{X} @var{command} @var{X} @var{shell_command}
+@itemx |  -d@var{X} @var{command} @var{X} @var{shell_command}
+Executes @var{command} and sends its output to @var{shell_command}.
+Note that no space is needed around @code{|}.
+If no @var{command} is provided, the last command executed is repeated.
+
+If the @var{command} contains a @code{|}, then the option @code{-d@var{X}}
+can be used to specify an alternate character that separates
+the  @var{command} from the @var{shell_command}.
+
+Example:
+@smallexample
+(gdb) pipe p full|wc
+      5      17      81
+(gdb) |p full|wc -l
+5
+(gdb) p full
+$4 = (black => 144,
+  red => 233,
+  green => 377,
+  blue => 610,
+  white => 987)
+(gdb) ||grep red
+  red => 233,
+(gdb) | -d! echo this contains a | char\n ! sed -e 's/|/PIPE/'
+this contains a PIPE char
+(gdb)
+@end smallexample
+@end table
+
 @node Logging Output
 @section Logging Output
 @cindex logging @value{GDBN} output
@@ -1482,6 +1519,8 @@ Set @code{redirect} if you want output to go only to the log file.
 Show the current values of the logging settings.
 @end table
 
+You can also redirect the output of a @value{GDBN} command to a
+shell command.  @xref{pipe}.
 @node Commands
 @chapter @value{GDBN} Commands
 
-- 
2.20.1

Re: [RFAv2 6/6] NEWS and documentation for | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Fri, 26 Apr 2019 22:11:08 +0200
> 
> +@item pipe [@var{command}] | @var{shell_command}
> +@itemx | [@var{command}] | @var{shell_command}
> +@item pipe -d@var{X} @var{command} @var{X} @var{shell_command}
> +@itemx |  -d@var{X} @var{command} @var{X} @var{shell_command}

All but the first @item should be @itemx.

> +If the @var{command} contains a @code{|}, then the option @code{-d@var{X}}
> +can be used to specify an alternate character that separates
> +the  @var{command} from the @var{shell_command}.

You didn't explain what X stands for.  I suggest to say this instead:

  ... to specify an alternate character @var{X} that separates ...

Also, our convention is not to upcase the argument of @var; it is
upcased in the produced Info manual, but has a different typeface in
other formats.

> +@smallexample
> +(gdb) pipe p full|wc
> +      5      17      81
> +(gdb) |p full|wc -l
> +5
> +(gdb) p full
> +$4 = (black => 144,
> +  red => 233,
> +  green => 377,
> +  blue => 610,
> +  white => 987)
> +(gdb) ||grep red
> +  red => 233,
> +(gdb) | -d! echo this contains a | char\n ! sed -e 's/|/PIPE/'
> +this contains a PIPE char
> +(gdb)
> +@end smallexample

This @example is very long, and could be split between two pages.  In
order to give TeX a hint where to split it, use @group, like this:

  @smallexample
  @group
  (gdb) pipe p full|wc
	5      17      81
  (gdb) |p full|wc -l
  5
  @end group
  @group
  (gdb) p full
  $4 = (black => 144,
    red => 233,
    green => 377,
    blue => 610,
    white => 987)
  (gdb) ||grep red
    red => 233,
  @end group
  @group
  (gdb) | -d! echo this contains a | char\n ! sed -e 's/|/PIPE/'
  this contains a PIPE char
  (gdb)
  @end group
  @end smallexample

(Feel free to divide differently into groups, but the principle is:
make a group of lines that you don't want split between pages.)

The documentation parts are OK with these nits fixed.

Thanks.

[RFAv2 5/6] Test the | (pipe) command.

[Philippe Waroquiers]

gdb/testsuite/ChangeLog
2019-04-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.base/shell.exp: Test pipe command.
---
 gdb/testsuite/gdb.base/shell.exp | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 60d6e31e4f..30516286cf 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -13,7 +13,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Test that the shell and ! commands work.
+# Test that the shell and ! and | and pipe commands work.
 
 gdb_exit
 gdb_start
@@ -22,3 +22,31 @@ gdb_test "shell echo foo" "foo"
 
 gdb_test "! echo foo" "foo"
 gdb_test "!echo foo" "foo"
+
+gdb_test "pipe help pipe | wc -l" "10" "check simple pipe"
+gdb_test "pipe help pipe | grep Usage: | wc -l" "4" "check double pipe"
+
+gdb_test "| help pipe | grep Usage: | wc -l" "4" "check double pipe, pipe char"
+gdb_test "|help pipe|grep Usage:|wc -l" "4" "no space around pipe char"
+
+gdb_test "echo coucou\\n" "coucou" "echo coucou"
+gdb_test "||wc -l" "1" "Check repeat previous command"
+
+gdb_test "| -d! echo this contains a | character\\n ! sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate separator"
+
+gdb_test "|-d! echo this contains a | character\\n!sed -e 's/|/PIPE/'" \
+    "this contains a PIPE character" "verify alternate separator, no space"
+
+# Some error handling verifications.
+gdb_test "|" "Missing \\\"\[COMMAND\] | SHELL_COMMAND\\\"" "all missing"
+gdb_test "| echo coucou" \
+    "Missing | SHELL_COMMAND in \\\"| COMMAND | SHELL_COMMAND\\\"" \
+    "pipe SHELL_COMMAND missing"
+gdb_test "|-d echo coucou" \
+    "Missing separator X after -d in \\\"| -dX COMMAND X SHELL_COMMAND\\\"" \
+    "separator missing"
+gdb_test "|-d! echo coucou" \
+    "Missing ! SHELL_COMMAND in \\\"| -d! COMMAND ! SHELL_COMMAND\\\"" \
+    "separator SHELL_COMMAND missing"
+
-- 
2.20.1

[RFAv2 1/6] Add previous_saved_command_line to allow a command to repeat a previous command.

[Philippe Waroquiers]

Currently, a previous command can be repeated when the user types an
empty line.  This is implemented in handle_line_of_input by
returning saved_command_line in case an empty line has been input.

If we want a command to repeat the previous command, we need to save
the previous saved_command_line, as when a command runs, the saved_command_line
already contains the current command line of the command being executed.

As suggested by Tom, the previous_saved_command_line is made static.
At the same time, saved_command_line is also made static.
The support functions/variables for the repeat command logic are now all
located inside top.c.

gdb/ChangeLog
2019-04-26  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* top.h (saved_command_line): Remove declaration.
	* top.c (previous_saved_command_line, previous_repeat_arguments):
	New variables.
	(saved_command_line): Make static, define together with other
	'repeat variables'.
	(dont_repeat): Clear repeat_arguments.
	(repeat_previous, get_saved_command_line, save_command_line):
	New functions.
	(gdb_init): Initialize saved_command_line
	and previous_saved_command_line.
	* main.c (captured_main_1): Remove saved_command_line initialization.
	* event-top.c (handle_line_of_input): Update to use
	the new 'repeat' related functions instead of direct access to
	saved_command_line.
	* command.h (repeat_previous, get_saved_command_line,
	save_command_line): New declarations.
	(dont_repeat): Add comment.
---
 gdb/command.h   | 36 ++++++++++++++++++++++++-
 gdb/event-top.c | 16 +++++------
 gdb/main.c      |  2 --
 gdb/top.c       | 70 +++++++++++++++++++++++++++++++++++++++++--------
 gdb/top.h       |  1 -
 5 files changed, 101 insertions(+), 24 deletions(-)

diff --git a/gdb/command.h b/gdb/command.h
index 4a239a7196..ea3a58f81c 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -448,7 +448,29 @@ extern void cmd_show_list (struct cmd_list_element *, int, const char *);
 
 extern void error_no_arg (const char *) ATTRIBUTE_NORETURN;
 
-extern void dont_repeat (void);
+
+/* Command line saving and repetition.
+   Each input line executed is saved for possibly be repeated either
+   when the user types an empty line, or be repeated by a command
+   that wants to repeat the previously executed command.  The below
+   functions are controlling command repetition.  */
+
+/* Commands call dont_repeat if they do not want to be repeated by null
+   lines or by repeat_previous ().  */
+
+extern void dont_repeat ();
+
+/* Command call repeat_previous if they want to repeat the previous command.
+   Such commands repeating the previous command must indicate
+   to not repeat themselves, to avoid recursive repeat.
+   repeat_previous will mark the current command as not repeating,
+   and will ensure get_saved_command_line returns the previous command,
+   so that the currently executing command can repeat it.  */
+
+extern void repeat_previous ();
+
+/* Prevent dont_repeat from working, and return a cleanup that
+   restores the previous state.  */
 
 extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
@@ -457,6 +479,18 @@ extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
 extern void set_repeat_arguments (const char *args);
 
+/* Returns the saved command line to repeat.
+   When a command is being executed, this is the currently executing
+   command line, unless the currently executing command has called
+   repeat_previous (): in this case, get_saved_command_line returns
+   the previously saved command line.  */
+
+extern char *get_saved_command_line ();
+
+/* Takes a copy of CMD, for possible repetition.  */
+
+extern void save_command_line (const char *cmd);
+
 /* Used to mark commands that don't do anything.  If we just leave the
    function field NULL, the command is interpreted as a help topic, or
    as a class of commands.  */
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 3ccf136ff1..b64b7462c7 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -634,11 +634,10 @@ command_line_append_input_line (struct buffer *cmd_line_buffer, const char *rl)
    If REPEAT, handle command repetitions:
 
      - If the input command line is NOT empty, the command returned is
-       copied into the global 'saved_command_line' var so that it can
-       be repeated later.
+       saved using save_command_line () so that it can be repeated later.
 
-     - OTOH, if the input command line IS empty, return the previously
-       saved command instead of the empty input line.
+     - OTOH, if the input command line IS empty, return the saved
+       command instead of the empty input line.
 */
 
 char *
@@ -673,7 +672,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   server_command = startswith (cmd, SERVER_COMMAND_PREFIX);
   if (server_command)
     {
-      /* Note that we don't set `saved_command_line'.  Between this
+      /* Note that we don't call `save_command_line'.  Between this
          and the check in dont_repeat, this insures that repeating
          will still do the right thing.  */
       return cmd + strlen (SERVER_COMMAND_PREFIX);
@@ -713,7 +712,7 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
     ;
   if (repeat && *p1 == '\0')
-    return saved_command_line;
+    return get_saved_command_line ();
 
   /* Add command to history if appropriate.  Note: lines consisting
      solely of comments are also added to the command history.  This
@@ -728,9 +727,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
-      xfree (saved_command_line);
-      saved_command_line = xstrdup (cmd);
-      return saved_command_line;
+      save_command_line (cmd);
+      return get_saved_command_line ();
     }
   else
     return cmd;
diff --git a/gdb/main.c b/gdb/main.c
index 35df1e497f..ef9bfe8fc6 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -499,8 +499,6 @@ captured_main_1 (struct captured_main_args *context)
 
   notice_open_fds ();
 
-  saved_command_line = (char *) xstrdup ("");
-
 #ifdef __MINGW32__
   /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
      as a Windows pipe, and Windows buffers on pipes.  */
diff --git a/gdb/top.c b/gdb/top.c
index 9a1c258d3f..93fed2ed80 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -134,8 +134,26 @@ show_confirm (struct ui_file *file, int from_tty,
 char *current_directory;
 
 /* The last command line executed on the console.  Used for command
-   repetitions.  */
-char *saved_command_line;
+   repetitions when the user enters an empty line.  */
+
+static char *saved_command_line;
+
+/* If not NULL, the arguments that should be passed if
+   saved_command_line is repeated.  */
+
+static const char *repeat_arguments;
+
+/* The previous last command line executed on the console.  Used for command
+   repetitions when a command want to relaunch the previously launched
+   command.  We need this as when a command is running, saved_command_line
+   already contains the line of the currently executing command.  */
+
+char *previous_saved_command_line;
+
+/* If not NULL, the arguments that should be passed if the
+   previous_saved_command_line is repeated.  */
+
+static const char *previous_repeat_arguments;
 
 /* Nonzero if the current command is modified by "server ".  This
    affects things like recording into the command history, commands
@@ -521,11 +539,6 @@ maybe_wait_sync_command_done (int was_sync)
     wait_sync_command_done ();
 }
 
-/* If not NULL, the arguments that should be passed if the current
-   command is repeated.  */
-
-static const char *repeat_arguments;
-
 /* See command.h.  */
 
 void
@@ -694,7 +707,7 @@ execute_command_to_string (const char *p, int from_tty)
 
 static int suppress_dont_repeat = 0;
 
-/* Commands call this if they do not want to be repeated by null lines.  */
+/* See command.h  */
 
 void
 dont_repeat (void)
@@ -708,11 +721,27 @@ dont_repeat (void)
      thing read from stdin in line and don't want to delete it.  Null
      lines won't repeat here in any case.  */
   if (ui->instream == ui->stdin_stream)
-    *saved_command_line = 0;
+    {
+      *saved_command_line = 0;
+      repeat_arguments = NULL;
+    }
+}
+
+/* See command.h  */
+
+void
+repeat_previous ()
+{
+  /* Do not repeat this command, as this command is a repeating command.  */
+  dont_repeat ();
+
+  /* We cannot free saved_command_line, as this line is being executed,
+     so swap it with previous_saved_command_line.  */
+  std::swap (previous_saved_command_line, saved_command_line);
+  std::swap (previous_repeat_arguments, repeat_arguments);
 }
 
-/* Prevent dont_repeat from working, and return a cleanup that
-   restores the previous state.  */
+/* See command.h.  */
 
 scoped_restore_tmpl<int>
 prevent_dont_repeat (void)
@@ -720,6 +749,22 @@ prevent_dont_repeat (void)
   return make_scoped_restore (&suppress_dont_repeat, 1);
 }
 
+char *
+get_saved_command_line ()
+{
+  return saved_command_line;
+}
+
+void
+save_command_line (const char *cmd)
+{
+  xfree (previous_saved_command_line);
+  previous_saved_command_line = saved_command_line;
+  previous_repeat_arguments = repeat_arguments;
+  saved_command_line = xstrdup (cmd);
+  repeat_arguments = NULL;
+}
+
 \f
 /* Read a line from the stream "instream" without command line editing.
 
@@ -2178,6 +2223,9 @@ The second argument is the terminal the UI runs on.\n"), &cmdlist);
 void
 gdb_init (char *argv0)
 {
+  saved_command_line = (char *) xstrdup ("");
+  previous_saved_command_line = (char *) xstrdup ("");
+
   if (pre_init_ui_hook)
     pre_init_ui_hook ();
 
diff --git a/gdb/top.h b/gdb/top.h
index 025d9389d6..aab03c13d6 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -217,7 +217,6 @@ extern void ui_register_input_event_handler (struct ui *ui);
 extern void ui_unregister_input_event_handler (struct ui *ui);
 
 /* From top.c.  */
-extern char *saved_command_line;
 extern int confirm;
 extern int inhibit_gdbinit;
 extern const char gdbinit[];
-- 
2.20.1