Re: [PATCH] An implementation of pipe to make I/O communication between gdb and shell.

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

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

On Thu, Aug 4, 2011 at 2:59 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Thursday 04 August 2011 08:51:14, Abhijit Halder wrote:
>> +  fstream = gdb_modify_io (gdb_stdio, pstream);
>> +  execute_command (pipe->gdb_cmd, from_tty);
>> +  pstream = gdb_modify_io (gdb_stdio, fstream);
>
> Looks like this leaves gdb_stdio in an inconsistent
> state if execute_command throws an error.
>
> Do you really need the new gdb_modify_io function?
> Can't ui_out_redirect (and stdio_file_new perhaps) do the job?
>
Yes you are right. I could not foresee this.

>> +      pipe->handle = popen (pipe->shell_cmd, pipe->mode);
>
> I'm not sure that'll build on all supported hosts.
> I think on Windows that may require use of _popen instead.
>
> "struct pipe_t" sounds like a recipe for system namespace
> colision (and _t is reserved for posix, though we
> have some precedent for abusing it), and is easily confused
> with the ser*.c pipe machinery.  Can you find an
> alternative name for the struct please?
> Perhaps struct pipe_cmd_state.
>
> --
> Pedro Alves
>

I have made the suggested corrections. Please review this.

Thanks,
Abhijit Halder

[-- Attachment #2: gdb-pipe-command.patch --]
[-- Type: text/x-patch, Size: 5647 bytes --]

diff -rup src/gdb/Makefile.in dst/gdb/Makefile.in
--- src/gdb/Makefile.in	2011-07-27 23:55:26.000000000 +0530
+++ dst/gdb/Makefile.in	2011-07-29 16:12:32.578048797 +0530
@@ -713,7 +713,7 @@ SFILES = ada-exp.y ada-lang.c ada-typepr
 	objc-exp.y objc-lang.c \
 	objfiles.c osabi.c observer.c osdata.c \
 	opencl-lang.c \
-	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c printcmd.c \
+	p-exp.y p-lang.c p-typeprint.c p-valprint.c parse.c pipe.c printcmd.c \
 	proc-service.list progspace.c \
 	prologue-value.c psymtab.c \
 	regcache.c reggroups.c remote.c remote-fileio.c reverse.c \
@@ -870,7 +870,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $
 	mi-common.o \
 	event-loop.o event-top.o inf-loop.o completer.o \
 	gdbarch.o arch-utils.o gdbtypes.o osabi.o copying.o \
-	memattr.o mem-break.o target.o parse.o language.o buildsym.o \
+	memattr.o mem-break.o target.o parse.o pipe.o language.o buildsym.o \
 	findcmd.o \
 	std-regs.o \
 	signals.o \
diff -rup src/gdb/pipe.c dst/gdb/pipe.c
--- src/gdb/pipe.c	2011-07-29 15:15:26.078048517 +0530
+++ dst/gdb/pipe.c	2011-08-05 13:10:51.411046880 +0530
@@ -0,0 +1,161 @@
+/* Everything about pipe, for GDB.
+
+   Copyright (C) 2011 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "defs.h"
+#include <ctype.h>
+#include "gdb_string.h"
+#include "ui-file.h"
+#include "ui-out.h"
+#include "cli/cli-utils.h"
+#include "gdbcmd.h"
+
+/* List of characters that can be used as delimiter to separate out
+   gdb-command and shell command.  */
+#define PIPE_DELIMITER "|/\\'\"`#@!$%<^>"
+
+/* The mode of stream operation.  */
+typedef char *iostream_mode_t;
+
+/* At present we support only write mode of operations to the pipe, i.e.,
+   gdb-command can only write to the pipe whose other terminal is owned by the
+   shell. In future we may start supporting read mode of operations as well.
+   But at present there is no need for that.  */
+#define WR_TEXT "w"
+
+struct pipe_object
+{
+  /* The shell-command.  */
+  char *shell_cmd;
+
+  /* The gdb-command.  */
+  char *gdb_cmd;
+
+  /* The delimiter to separate out gdb-command and shell-command.  */
+  char dlim;
+
+  /* The supported mode of stream operations on gdb-end pipe.  */
+  iostream_mode_t mode;
+
+  /* The gdb-end stream pointer to the pipe.  */
+  FILE *handle;
+};
+
+/* Prototype of local functions.  */
+
+static struct pipe_object *construct_pipe (char *);
+
+static void execute_command_to_pipe (struct pipe_object *, int);
+
+static void destruct_pipe (struct pipe_object *);
+
+static void pipe_command (char *, int);
+
+static struct pipe_object *
+construct_pipe (char *p)
+{
+  struct pipe_object *pipe = NULL;
+  struct cleanup *old_chain;
+
+  if (p != NULL && *p != '\0')
+    {
+      pipe = xmalloc (sizeof (struct pipe_object));
+      old_chain = make_cleanup (xfree, pipe);
+      pipe->mode = WR_TEXT;
+
+      p = skip_spaces (p);
+      pipe->dlim = *p++;
+      p = skip_spaces (p);
+      pipe->gdb_cmd = p;
+
+      /* Validate the delimiter from a pre-defined whitelist characters. This
+	 will enforce not to use special (e.g. alpha-numeric) characters.  */
+      /* NOTE: If DLIM become null, P starts pointing to a bad memory
+	 location, hence before doing further processing of P we should check
+	 DLIM.  */
+      if (pipe->dlim == '\0'
+	  || strchr (PIPE_DELIMITER, pipe->dlim) == NULL)
+	error (_("Invalid delimiter '%c'"), pipe->dlim);
+
+      if ((p = strchr (p, pipe->dlim)) == NULL)
+	error (_("Found no shell command"));
+
+      *p++ = '\0';
+      pipe->shell_cmd = p;
+
+      pipe->handle = popen (pipe->shell_cmd, pipe->mode);
+
+      if (!pipe->handle)
+	error (_("Failed to create pipe.\n%s"), strerror (errno));
+
+      discard_cleanups (old_chain);
+    }
+
+  return pipe;
+}
+
+static void
+execute_command_to_pipe (struct pipe_object *pipe, int from_tty)
+{
+  struct cleanup *cleanup;
+  struct ui_file *fp;
+
+  cleanup = set_batch_flag_and_make_cleanup_restore_page_info ();
+  fp = stdio_fileopen (pipe->handle);
+  make_cleanup_ui_file_delete (fp);
+  make_cleanup_restore_ui_file (&gdb_stdout);
+
+  if (ui_out_redirect (uiout, fp) < 0)
+    warning (_("Current output protocol does not support redirection"));
+  else 
+    make_cleanup_ui_out_redirect_pop (uiout);
+
+  gdb_stdout = fp;
+  execute_command (pipe->gdb_cmd, from_tty);
+  do_cleanups (cleanup);
+}
+
+static void
+destruct_pipe (struct pipe_object *pipe)
+{
+  pclose (pipe->handle);
+  xfree (pipe);
+}
+
+static void
+pipe_command (char *arg, int from_tty)
+{
+  struct pipe_object *pipe;
+
+  pipe = construct_pipe (arg);
+  if (pipe != NULL)
+    {
+      execute_command_to_pipe (pipe, from_tty);
+      destruct_pipe (pipe);
+    }
+}
+
+void
+_initialize_pipe (void)
+{
+  add_cmd ("pipe", no_class, pipe_command, _("\
+Create pipe to pass gdb-command output to the shell for processing.\n\
+Arguments are a delimiter character, followed by a gdb-command, \
+followed by a shell-command."),
+	   &cmdlist);
+}

[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 221 bytes --]

2011-07-29 Abhijit Halder <abhijit.k.halder@symantec.com>

	Implementation of pipe to make I/O communication 
	between gdb and shell.

	* pipe.c: New file.
	* Makefile.in (SFILES): Add pipe.c.
	(COMMON_OBS): Add pipe.o.