[RFA 0/4] Implement | (pipe) command.

[RFA 0/4] Implement | (pipe) command.

[Philippe Waroquiers]

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

The first patch allows a command to repeat a previous command.
Currently only used by the pipe command added in this series, but
the idea is that the slash command will also use this feature to
repeat a previous command.

[RFA 1/4] 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.

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

	* top.h (previous_saved_command_line): New declaration.
	* main.c (captured_main_1): Initialize previous_saved_command_line.
	* event-top.c (handle_line_of_input): Copy saved_command_line into
	previous_saved_command_line before replacing saved_command_line.
	* command.h (repeat_previous): New declaration.
	* top.c (previous_saved_command_line): New variable.
	(repeat_previous): New function.
---
 gdb/command.h   |  1 +
 gdb/event-top.c |  3 ++-
 gdb/main.c      |  1 +
 gdb/top.c       | 29 ++++++++++++++++++++++++++++-
 gdb/top.h       |  1 +
 5 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/gdb/command.h b/gdb/command.h
index 4a239a7196..20a182db0c 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -449,6 +449,7 @@ 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);
+extern void repeat_previous (void);
 
 extern scoped_restore_tmpl<int> prevent_dont_repeat (void);
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index cd54eb5a2c..6ff6203a0a 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -724,7 +724,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   /* Save into global buffer if appropriate.  */
   if (repeat)
     {
-      xfree (saved_command_line);
+      xfree (previous_saved_command_line);
+      previous_saved_command_line = saved_command_line;
       saved_command_line = xstrdup (cmd);
       return saved_command_line;
     }
diff --git a/gdb/main.c b/gdb/main.c
index e67efc7bcd..c6295ab500 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -500,6 +500,7 @@ captured_main_1 (struct captured_main_args *context)
   notice_open_fds ();
 
   saved_command_line = (char *) xstrdup ("");
+  previous_saved_command_line = (char *) xstrdup ("");
 
 #ifdef __MINGW32__
   /* Ensure stderr is unbuffered.  A Cygwin pty or pipe is implemented
diff --git a/gdb/top.c b/gdb/top.c
index 9a1c258d3f..b46330ff2c 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -134,8 +134,13 @@ show_confirm (struct ui_file *file, int from_tty,
 char *current_directory;
 
 /* The last command line executed on the console.  Used for command
-   repetitions.  */
+   repetitions when the user enters an empty line.  */
 char *saved_command_line;
+/* 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;
 
 /* Nonzero if the current command is modified by "server ".  This
    affects things like recording into the command history, commands
@@ -711,6 +716,28 @@ dont_repeat (void)
     *saved_command_line = 0;
 }
 
+/* Command call this 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 restore saved_command_line from the
+   previous_saved_command_line, so that the currently executing
+   command can repeat it by using saved_command_line.  */
+
+void
+repeat_previous (void)
+{
+  char *tmp = previous_saved_command_line;
+
+  /* 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.  */
+  previous_saved_command_line = saved_command_line;
+  saved_command_line = tmp;
+}
+
 /* Prevent dont_repeat from working, and return a cleanup that
    restores the previous state.  */
 
diff --git a/gdb/top.h b/gdb/top.h
index 025d9389d6..ff8cc10e55 100644
--- a/gdb/top.h
+++ b/gdb/top.h
@@ -218,6 +218,7 @@ extern void ui_unregister_input_event_handler (struct ui *ui);
 
 /* From top.c.  */
 extern char *saved_command_line;
+extern char *previous_saved_command_line;
 extern int confirm;
 extern int inhibit_gdbinit;
 extern const char gdbinit[];
-- 
2.20.1

Re: [RFA 1/4] Add previous_saved_command_line to allow a command to repeat a previous command.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> +/* The previous last command line executed on the console.  Used for command
Philippe> +   repetitions when a command want to relaunch the previously launched
Philippe> +   command.  We need this as when a command is running, saved_command_line
Philippe> +   already contains the line of the currently executing command.  */
Philippe> +char *previous_saved_command_line;

Is there a way to make this static?  Like have repeat_previous return
the desired value or something like that?

Philippe> +void
Philippe> +repeat_previous (void)

You can use () instead of (void) in C++.

Philippe> +  /* We cannot free saved_command_line, as this line is being executed,
Philippe> +     so swap it with previous_saved_command_line.  */
Philippe> +  previous_saved_command_line = saved_command_line;
Philippe> +  saved_command_line = tmp;

Just use std::swap here.

Tom

[RFA 2/4] 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
  Executes COMMAND and sends its output to 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-20  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   | 91 ++++++++++++++++++++++++++++++++++++++++++++
 gdb/cli/cli-decode.c |  4 +-
 2 files changed, 93 insertions(+), 2 deletions(-)

diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 5f3b973f06..6964f9e1ab 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -24,6 +24,8 @@
 #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 "gdbthread.h"
 #include "gdb_regex.h"	/* Used by apropos_command.  */
 #include "gdb_vfork.h"
 #include "linespec.h"
@@ -854,6 +856,86 @@ 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)
+{
+  FILE *to;
+  const char *shell_command = arg;
+
+  if (arg == NULL)
+    error (_("Missing \"[COMMAND] | SHELL_COMMAND\""));
+
+  while (*shell_command && *shell_command != '|')
+    shell_command++;
+
+  if (*shell_command != '|')
+    error (_("Missing | SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
+
+  arg = skip_spaces (arg);
+  std::string gdb_cmd (arg, shell_command - arg);
+
+  if (gdb_cmd.length () == 0)
+    {
+      repeat_previous ();
+      gdb_cmd = skip_spaces (saved_command_line);
+      if (gdb_cmd.length () == 0)
+	error (_("No previous command to relaunch"));
+    }
+
+  shell_command++; /* skip the |.  */
+  shell_command = skip_spaces (shell_command);
+  if (*shell_command == 0)
+    error (_("Missing SHELL_COMMAND in \"| [COMMAND] | SHELL_COMMAND\""));
+
+  to = popen (shell_command, "w");
+  if (to == NULL)
+    error (_("Error launching \"%s\""), shell_command);
+
+  try
+    {
+      std::string gdb_cmd_result;
+      {
+	/* In case GDB_CMD switches of inferior/thread/frame, the below
+	   restores the inferior/thread/frame.  */
+	scoped_restore_current_thread restore;
+
+	gdb_cmd_result = execute_command_to_string (gdb_cmd.c_str (),
+						    from_tty);
+
+	if (fwrite (gdb_cmd_result.c_str (), 1, gdb_cmd_result.length (), to)
+	    != gdb_cmd_result.length ())
+	  {
+	    error (_("error writing \"%s\" result to \"%s\", errno %s"),
+		   gdb_cmd.c_str (), shell_command, safe_strerror (errno));
+	  }
+      }
+    }
+  catch (const gdb_exception_error &ex)
+    {
+      pclose (to);
+      throw;
+    }
+
+  int shell_command_status = pclose (to);
+  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 +1927,15 @@ 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\
+Executes COMMAND and sends its output to 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: [RFA 2/4] Implement | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat, 20 Apr 2019 23:21:51 +0200
> 
> +  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));

These macros will need to be ported to MinGW, since the current
definitions in gdb_wait.h are for Posix systems (and never used on
anything other than that).  Gnulib has replacements, but they are less
functional on Windows than I'd like them to be, in particular when the
shell program exited due to a signal.  I can provide better
replacements if you want.

Re: [RFA 2/4] Implement | (pipe) command.

[Philippe Waroquiers]

On Sun, 2019-04-21 at 08:19 +0300, Eli Zaretskii wrote:
> > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Date: Sat, 20 Apr 2019 23:21:51 +0200
> > 
> > +  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));
> 
> These macros will need to be ported to MinGW, since the current
> definitions in gdb_wait.h are for Posix systems (and never used on
> anything other than that).  Gnulib has replacements, but they are less
> functional on Windows than I'd like them to be, in particular when the
> shell program exited due to a signal.  I can provide better
> replacements if you want.
It would be nice to have better replacement for Windows,
as I do not have the knowledge and needed setup to test
for this platform.

Thanks for the review.

Philippe

Re: [RFA 2/4] Implement | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: gdb-patches@sourceware.org
> Date: Sun, 21 Apr 2019 11:40:22 +0200
> 
> > > +  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));
> > 
> > These macros will need to be ported to MinGW, since the current
> > definitions in gdb_wait.h are for Posix systems (and never used on
> > anything other than that).  Gnulib has replacements, but they are less
> > functional on Windows than I'd like them to be, in particular when the
> > shell program exited due to a signal.  I can provide better
> > replacements if you want.
> It would be nice to have better replacement for Windows,
> as I do not have the knowledge and needed setup to test
> for this platform.

Here's my proposal:

#define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
#define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) == 0xC0000000)
#define WEXITSTATUS(stat_val) ((stat_val) & 255)
#define WTERMSIG(stat_val)    windows_status_to_termsig (stat_val)

where windows_status_to_termsig should use the xlate[] array defined
in windows-nat.c, like the (ifdef'ed away) code in
windows_nat_target::resume does.

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 above 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 I think such a
negligibly small probability of false positives is justified by the
utility of reporting the terminating signal in the "normal" cases.

Re: [RFA 2/4] Implement | (pipe) command.

[Philippe Waroquiers]

On Sun, 2019-04-21 at 13:18 +0300, Eli Zaretskii wrote:
> Here's my proposal:
> 
> #define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
> #define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) == 0xC0000000)
> #define WEXITSTATUS(stat_val) ((stat_val) & 255)
> #define WTERMSIG(stat_val)    windows_status_to_termsig (stat_val)
> 
> where windows_status_to_termsig should use the xlate[] array defined
> in windows-nat.c, like the (ifdef'ed away) code in
> windows_nat_target::resume does.
> 
> 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 above 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 I think such a
> negligibly small probability of false positives is justified by the
> utility of reporting the terminating signal in the "normal" cases.
Thanks for the above.

To confirm what to do, I will try to explain my current understanding:

Currently, on MingW, the above macros (WIFEXITED, WIFSIGNALED, ...)
are not defined.

So, the idea is to have gdb_wait.h defining them on MingW,
with something like:

#ifndef	WIFEXITED
#if defined (__MINGW32__)
#define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
#else
#define WIFEXITED(w)	(((w)&0377) == 0)
#endif
#endif

Then in windows-nat.c, implement windows_status_to_termsig
that searches in xlate for stat_val & ~0xC0000000 to give
the equivalent signal.

Does the above look reasonable ?

Thanks

Philippe

Re: [RFA 2/4] Implement | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 22 Apr 2019 12:30:00 +0200
> 
> Currently, on MingW, the above macros (WIFEXITED, WIFSIGNALED, ...)
> are not defined.

I think gdb_wait.h does define them for MinGW, it's just that those
definitions are currently unused anywhere that is compiled into the
MinGW build.

> So, the idea is to have gdb_wait.h defining them on MingW,
> with something like:
> 
> #ifndef	WIFEXITED
> #if defined (__MINGW32__)
> #define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
> #else
> #define WIFEXITED(w)	(((w)&0377) == 0)
> #endif
> #endif
> 
> Then in windows-nat.c, implement windows_status_to_termsig
> that searches in xlate for stat_val & ~0xC0000000 to give
> the equivalent signal.
> 
> Does the above look reasonable ?

Yes, thanks.

Re: [RFA 2/4] Implement | (pipe) command.

[Philippe Waroquiers]

On Mon, 2019-04-22 at 14:08 +0300, Eli Zaretskii wrote:
> > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Cc: gdb-patches@sourceware.org
> > Date: Mon, 22 Apr 2019 12:30:00 +0200
> > 
> > Currently, on MingW, the above macros (WIFEXITED, WIFSIGNALED, ...)
> > are not defined.
> 
> I think gdb_wait.h does define them for MinGW, it's just that those
> definitions are currently unused anywhere that is compiled into the
> MinGW build.
> 
> > So, the idea is to have gdb_wait.h defining them on MingW,
> > with something like:
> > 
> > #ifndef	WIFEXITED
> > #if defined (__MINGW32__)
> > #define WIFEXITED(stat_val)   (((stat_val) & 0xC0000000) == 0)
> > #else
> > #define WIFEXITED(w)	(((w)&0377) == 0)
> > #endif
> > #endif
> > 
> > Then in windows-nat.c, implement windows_status_to_termsig
> > that searches in xlate for stat_val & ~0xC0000000 to give
> > the equivalent signal.
> > 
> > Does the above look reasonable ?
> 
> Yes, thanks.
So, here is a trial patch, only compiled on Debian/amd64.
Does that compile/work on MingW ?
Thanks
Philippe

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..e12cf10416 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>
 {

Re: [RFA 2/4] Implement | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: gdb-patches@sourceware.org
> Date: Mon, 22 Apr 2019 15:15:44 +0200
> 
> So, here is a trial patch, only compiled on Debian/amd64.
> Does that compile/work on MingW ?

Thanks, it looks good when I read it.  I won't be able to actually
test it in compiling the MinGW build for a few days, perhaps someone
else will be able to do so sooner.

Re: [RFA 2/4] Implement | (pipe) command.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> +  while (*shell_command && *shell_command != '|')
Philippe> +    shell_command++;

I think using strchr would be preferable here.

Philippe> +  if (gdb_cmd.length () == 0)

Use .empty instead.

Philippe> +      if (gdb_cmd.length () == 0)

Likewise.

Philippe> +  to = popen (shell_command, "w");

I wonder if it's better to use libiberty's pexecute code?
This may avoid the WIFEXITED problems.

Philippe> +	/* In case GDB_CMD switches of inferior/thread/frame, the below
Philippe> +	   restores the inferior/thread/frame.  */
Philippe> +	scoped_restore_current_thread restore;

What's the rationale for this?

Philippe> +	gdb_cmd_result = execute_command_to_string (gdb_cmd.c_str (),
Philippe> +						    from_tty);
Philippe> +
Philippe> +	if (fwrite (gdb_cmd_result.c_str (), 1, gdb_cmd_result.length (), to)
Philippe> +	    != gdb_cmd_result.length ())
Philippe> +	  {
Philippe> +	    error (_("error writing \"%s\" result to \"%s\", errno %s"),
Philippe> +		   gdb_cmd.c_str (), shell_command, safe_strerror (errno));
Philippe> +	  }

Maybe it would be preferable to redirect gdb's actual output here
instead, using whatever mechanism it is that "set logging" uses.

Tom

Re: [RFA 2/4] Implement | (pipe) command.

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Date: Wed, 24 Apr 2019 14:50:47 -0600
> 
> Philippe> +  to = popen (shell_command, "w");
> 
> I wonder if it's better to use libiberty's pexecute code?
> This may avoid the WIFEXITED problems.

Libiberty's pexecute doesn't solve the WIFEXITED problem, AFAICT, it
just returns the exit status to the caller, and I see no facilities
there to help interpret that status with platform-independent code.
Did I miss something?

[RFA 3/4] Test the | (pipe) command.

[Philippe Waroquiers]

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

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

diff --git a/gdb/testsuite/gdb.base/shell.exp b/gdb/testsuite/gdb.base/shell.exp
index 60d6e31e4f..1840eab322 100644
--- a/gdb/testsuite/gdb.base/shell.exp
+++ b/gdb/testsuite/gdb.base/shell.exp
@@ -22,3 +22,12 @@ gdb_test "shell echo foo" "foo"
 
 gdb_test "! echo foo" "foo"
 gdb_test "!echo foo" "foo"
+
+gdb_test "pipe help pipe | wc -l" "6" "check simple pipe"
+gdb_test "pipe help pipe | grep Usage: | wc -l" "2" "check double pipe"
+
+gdb_test "| help pipe | grep Usage: | wc -l" "2" "check double pipe, pipe char"
+gdb_test "|help pipe|grep Usage:|wc -l" "2" "no space around pipe char"
+
+gdb_test "echo coucou\\n" "coucou" "echo coucou"
+gdb_test "||wc -l" "1" "Check repeat previous command"
-- 
2.20.1

Re: [RFA 3/4] Test the | (pipe) command.

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

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

Philippe> 	* gdb.base/shell.exp: Test pipe command.

This seems fine though I wonder whether there are test environments
without the necessary commands.

Tom

[RFA 4/4] 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 | 31 +++++++++++++++++++++++++++++++
 2 files changed, 39 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 a3a5f3e28c..0350958d4e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -1454,6 +1454,35 @@ 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}
+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.
+
+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) 
+@end smallexample
+@end table
+
 @node Logging Output
 @section Logging Output
 @cindex logging @value{GDBN} output
@@ -1482,6 +1511,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.  See @ref{pipe}.
 @node Commands
 @chapter @value{GDBN} Commands
 
-- 
2.20.1

Re: [RFA 4/4] NEWS and documentation for | (pipe) command.

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat, 20 Apr 2019 23:21:53 +0200
> 
> 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.
> +

This is OK.

> +You can also redirect the output of a @value{GDBN} command to a
> +shell command.  See @ref{pipe}.
                   ^^^^^^^^^^^^^^
This isn't incorrect, bur @xref{pipe} is better.

Otherwise, the documentation part is OK, thanks.

Re: [RFA 0/4] Implement | (pipe) command.

[Abhijit Halder]

Hi Philippe,

If I see it correctly, the syntax of the command is
pipe <gdb command> | <shell command>
If the gdb command contains '|' the above syntax will be a problem.
I suggest below syntax
pipe <delimiter> <gdb command> <delimiter> <shell command>
Here the advantage is the delimiter can be anything and conveniently be
chosen by the user.

I made a similar attempt few years back.
https://sourceware.org/ml/gdb-patches/2012-01/msg00098.html

This you can take this as a reference.

Thanks,
Abhijit Halder



On Sun, Apr 21, 2019 at 2:52 AM Philippe Waroquiers <
philippe.waroquiers@skynet.be> wrote:

> This patch series adds the pipe command, that allows to send the output
> of a GDB command to a shell command.
>
> The first patch allows a command to repeat a previous command.
> Currently only used by the pipe command added in this series, but
> the idea is that the slash command will also use this feature to
> repeat a previous command.
>
>
>

Re: [RFA 0/4] Implement | (pipe) command.

[Philippe Waroquiers]

Hello Abhijit,

Yes, the fact that the command line interface of GDB has
no 'formal grammar' is sometimes making things difficult.

In this case, I thought it was not particularly important
to support | characters in the gdb command, but I might be wrong.

If we want to handle this case, then I would prefer
to have an optional argument: e.g. pipe [-dX] ... would indicate
to use the character X as delimiter between the GDB command
and the shell command.
This syntax is lightweight, and still allows by default to
re-run the previous command with
   || some_shell_command

In any case, I agree that either such case should be properly
handled e.g. with [-dX] or at least the limitation should
be documented in the user manual.

Thanks for the feedback,

Philippe


On Tue, 2019-04-23 at 02:14 +0530, Abhijit Halder wrote:
> Hi Philippe,
> 
> If I see it correctly, the syntax of the command is
> pipe <gdb command> | <shell command>
> If the gdb command contains '|' the above syntax will be a problem.
> I suggest below syntax 
> pipe <delimiter> <gdb command> <delimiter> <shell command>
> Here the advantage is the delimiter can be anything and conveniently be chosen by the user.
> 
> I made a similar attempt few years back.
> https://sourceware.org/ml/gdb-patches/2012-01/msg00098.html
> 
> This you can take this as a reference.
> 
> Thanks,
> Abhijit Halder
> 
> 
> 
> On Sun, Apr 21, 2019 at 2:52 AM Philippe Waroquiers <philippe.waroquiers@skynet.be> wrote:
> > This patch series adds the pipe command, that allows to send the output
> > of a GDB command to a shell command.
> > 
> > The first patch allows a command to repeat a previous command.
> > Currently only used by the pipe command added in this series, but
> > the idea is that the slash command will also use this feature to
> > repeat a previous command.
> > 
> > 
> > 

Re: [RFA 0/4] Implement | (pipe) command.

[Tom Tromey]

>>>>> "Abhijit" == Abhijit Halder <abhijit.k.halder@gmail.com> writes:

Abhijit> If I see it correctly, the syntax of the command is
Abhijit> pipe <gdb command> | <shell command>
Abhijit> If the gdb command contains '|' the above syntax will be a problem.

Yeah, "print x | y" or other expressions come to mind.

Tom