Re: [PATCH] gdb output pipelining to shell

[Abhijit Halder <abhijit.k.halder@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4055 bytes --]

Since I was overwriting the braces by space character, this could
essentially change a valid gdb command having braces after pipe.
Here is the example:
(gdb) p argc | {int} &argc

I have modified the patch to defer the any overwrite (and essentially
the modification) of actual input string for very last moment when the
a sub-string of a given input in gdb prompt is qualified to be in a
valid format of shell command.

I am re-submitting the patch for review and comments with the
necessary corrections in place. Please do comment on the changes.

 top.c     |   77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 ui-file.c |   15 ++++++++++++
 ui-file.h |    3 ++
 3 files changed, 94 insertions(+), 1 deletion(-)

Regards,
Abhijit Halder


On Mon, Jul 18, 2011 at 11:31 PM, Abhijit Halder
<abhijit.k.halder@gmail.com> wrote:
> Please don't consider this patch. This has a conflict with following
> syntax of of
> (gdb) p var | {type} address
>
> On Sat, Jul 16, 2011 at 11:35 PM, Abhijit Halder
> <abhijit.k.halder@gmail.com> wrote:
>> A small correction. Re-submitting the patch. The earlier patch was not
>> able to handle below situation:
>> (gdb) p '|' | { less }
>>
>> On Sat, Jul 16, 2011 at 3:38 PM, Abhijit Halder
>> <abhijit.k.halder@gmail.com> wrote:
>>> On Sat, Jul 16, 2011 at 3:17 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>>> Date: Sat, 16 Jul 2011 14:12:35 +0530
>>>>> From: Abhijit Halder <abhijit.k.halder@gmail.com>
>>>>>
>>>>> I have implemented a feature which will allow one to pass the output
>>>>> of any gdb command to the shell for further processing.
>>>>
>>>> Thanks.
>>>>
>>>> If this is accepted, we will need a corresponding addition to the
>>>> manual.
>>>>
>>>>> +      for (cpos = spos; (cpos = memchr (cpos, '"', (sh_cmd-cpos))) != NULL; cpos++)
>>>>> +        quote_cnt++;
>>>>> +      spos = (sh_cmd + 1);
>>>>> +      if ((quote_cnt % 2) == 0 || (sh_cmd = strchr (spos, '|')) == NULL)
>>>>> +        break;
>>>>
>>>> I'm not sure I understand this (comments would be helpful).  Are you
>>>> assuming that quote characters `"' in shell commands cannot be
>>>> escaped, e.g. with a backslash?  And what about quoting with a single
>>>> quote character ("'")?
>>>>
>>> Any pipe ('|' character), not within double quote will be considered
>>> as either a bitwise-OR operator or a pipe between gdb and shell.
>>> String after pipe (not within double quote) will be considered as
>>> shell command if (and only if) it is encapsulated within opening and
>>> closing braces ('{' and '}') . The shell command can surely contain
>>> double quote, even braces. There is no validation done for shell
>>> command.
>>>
>>>>> +    if (*cpos != '{')
>>>>> +      return NULL;
>>>>> +
>>>>> +    *cpos = ' ';
>>>>> +
>>>>> +    cpos = epos;
>>>>> +    while (isspace(*cpos))
>>>>> +      cpos--;
>>>>> +
>>>>> +    if (*cpos != '}')
>>>>> +      return NULL;
>>>>
>>>> What is this magic about {...} that you are removing?  Again, comments
>>>> could help.
>>>>
>>> Here I am removing the braces from the shell command. An example will
>>> help in understanding this:
>>> (gdb) thread apply all bt | { grep "foo" }
>>> This will be a valid command. { grep "foo" } will be considered as
>>> shell command and we need to erase the braces part of it to make it a
>>> valid shell command.
>>>>> +
>>>>> +    *cpos = ' ';
>>>>> +    }
>>>>> +
>>>>> +  if (sh_cmd)
>>>>> +    *sh_cmd++ = '\0';
>>>>> +
>>>>> +  return sh_cmd;
>>>>
>>>> This butchers the string passed to execute_command.  Are you sure all
>>>> the callers of execute_command can safely deal with that?  What if the
>>>> string is a constant string, for example?
>>>>
>>> The new code path will be executed only when one will enter a command
>>> containing pipeline between gdb and shell. In that case the string
>>> passed to execute_command must not be a constant string (since it is
>>> user input from gdb prompt). Hence we are safe.
>>>
>>
>

[-- Attachment #2: gdb-shell-integration-correction.patch --]
[-- Type: text/x-patch, Size: 3543 bytes --]

diff -rup src//gdb/top.c dst//gdb/top.c
--- src//gdb/top.c	2011-06-28 00:51:50.000000000 +0530
+++ dst//gdb/top.c	2011-07-19 00:10:51.623144702 +0530
@@ -48,6 +48,7 @@
 #include "event-loop.h"
 #include "gdbthread.h"
 #include "python/python.h"
+#include "ui-file.h"
 
 /* readline include files.  */
 #include "readline/readline.h"
@@ -358,6 +359,71 @@ prepare_execute_command (void)
   return cleanup;
 }
 
+char *
+parse_for_shell_command (char *p)
+{
+  char *sh_cmd, *cpos, *spos, *epos;
+  int quote_cnt = 0;
+
+  if ((sh_cmd = strchr (p, '|')) != NULL)
+    {
+      spos = p;
+      epos = p + strlen (p) - 1;
+
+      for (;;)
+    {
+      for (cpos = spos; (cpos = memchr (cpos, '"', (sh_cmd-cpos))) != NULL; cpos++)
+        quote_cnt++;
+      spos = (sh_cmd + 1);
+      if ((quote_cnt % 2) == 0 || (sh_cmd = strchr (spos, '|')) == NULL)
+        break;
+    }
+
+    if (sh_cmd == NULL)
+      return NULL;
+
+    cpos = spos;
+    while (isspace(*cpos))
+      cpos++;
+
+    if (*cpos != '{')
+      return parse_for_shell_command (cpos);
+
+    spos = cpos;
+
+    cpos = epos;
+    while (isspace(*cpos))
+      cpos--;
+
+    if (*cpos != '}')
+      return parse_for_shell_command (cpos);
+
+    epos = cpos;
+    }
+
+  if (sh_cmd)
+    {
+      *spos = *epos = ' ';
+      *sh_cmd++ = '\0';
+    }
+
+  return sh_cmd;
+}
+
+/* Run execute_command for P and FROM_TTY. Write output in pipe, 
+   do not display it to the screen.  */
+
+void
+execute_command_to_pipe (char *p, int from_tty, FILE *pipe)
+{
+  FILE *file;
+
+  file = gdb_modify_io (gdb_stdout, pipe);
+  execute_command (p, from_tty);
+  pipe = gdb_modify_io (gdb_stdout, file);
+  pclose (pipe);
+}
+
 /* Execute the line P as a command, in the current user context.
    Pass FROM_TTY as second argument to the defining function.  */
 
@@ -368,7 +434,16 @@ execute_command (char *p, int from_tty)
   struct cmd_list_element *c;
   enum language flang;
   static int warned = 0;
-  char *line;
+  char *line, *sh_cmd;
+
+  if ((sh_cmd = parse_for_shell_command (p)) != NULL) 
+    {
+      FILE *pipe;
+
+      pipe = popen (sh_cmd, "w");
+      execute_command_to_pipe (p, from_tty, pipe);
+      return;
+    }
 
   cleanup = prepare_execute_command ();
 
diff -rup src//gdb/ui-file.c dst//gdb/ui-file.c
--- src//gdb/ui-file.c	2011-05-14 11:14:36.000000000 +0530
+++ dst//gdb/ui-file.c	2011-07-15 13:49:34.999910332 +0530
@@ -617,6 +617,21 @@ struct ui_file *
 stdio_fileopen (FILE *file)
 {
   return stdio_file_new (file, 0);
+
+}
+
+FILE *
+gdb_modify_io (struct ui_file *file, FILE *iostream_new)
+{
+  FILE *iostream_old;
+  struct stdio_file *stdio = ui_file_data (file);
+
+  if (stdio->magic != &stdio_file_magic)
+    internal_error (__FILE__, __LINE__,
+		    _("gdb_modify_io: bad magic number"));
+  iostream_old = stdio->file;
+  stdio->file = iostream_new;
+  return iostream_old;
 }
 
 struct ui_file *
diff -rup src//gdb/ui-file.h dst//gdb/ui-file.h
--- src//gdb/ui-file.h	2011-05-13 22:58:20.000000000 +0530
+++ dst//gdb/ui-file.h	2011-07-15 13:48:54.603912199 +0530
@@ -126,6 +126,9 @@ extern struct ui_file *stdio_fileopen (F
 /* Open NAME returning an STDIO based UI_FILE.  */
 extern struct ui_file *gdb_fopen (char *name, char *mode);
 
+/* Modify the file pointer of an STDIO based UI_FILE. */
+FILE *gdb_modify_io (struct ui_file *file, FILE *iostream_new);
+
 /* Create a file which writes to both ONE and TWO.  CLOSE_ONE
    and CLOSE_TWO indicate whether the original files should be
    closed when the new file is closed.  */