Re: "target remote | " stderr

[Vladimir Prus <ghost@cs.msu.su>]

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

Daniel Jacobowitz wrote:

> On Wed, Jan 31, 2007 at 05:34:21PM +0300, Vladimir Prus wrote:
>> Implementing for MinGW required modifying libiberty so that
>> it can catch stderr to a pipe. Here's a patch for gdb that
>> relies on the libiberty patch. Does this sound OK
>> provided libiberty patch is approved?
> 
> Do we automatically get a non-blocking pipe on mingw, or do we have to
> set it ourselves?  

We have to set it ourself, unfortunately. I had one test case working
fine, but got gdb hanging on some other.

The attached patch works around the blocking issues, and more extensively
tested. OK?

- Volodya

        Pass stderr of program run with "target remote |"
        via gdb_stderr.
        * serial.c (serial_open): Set error_fd to -1.
        * serial.h (struct serial): New field error_fd.
        * ser-pipe.c (pipe_open): Create another pair
        of sockets.  Pass stderr to gdb.
        * ser-mingw.c (pipe_windows_open): Pass
        PEX_STDERR_TO_PIPE to pex_run.  Initialize
        sd->error_fd.
        * ser-base.c (generic_readchar): Check if there's
        anything in stderr channel and route that to gdb_stderr.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: remote_stderr_try2__gdb_mainline.diff --]
[-- Type: text/x-diff; name="remote_stderr_try2__gdb_mainline.diff", Size: 5688 bytes --]

--- gdb/serial.c	(/mirrors/gdb_mainline)	(revision 3417)
+++ gdb/serial.c	(/patches/gdb/remote_stderr_try2/gdb_mainline)	(revision 3417)
@@ -211,6 +211,7 @@ serial_open (const char *name)
 
   scb->bufcnt = 0;
   scb->bufp = scb->buf;
+  scb->error_fd = -1;
 
   if (scb->ops->open (scb, open_name))
     {
--- gdb/serial.h	(/mirrors/gdb_mainline)	(revision 3417)
+++ gdb/serial.h	(/patches/gdb/remote_stderr_try2/gdb_mainline)	(revision 3417)
@@ -191,6 +191,12 @@ extern int serial_debug_p (struct serial
 struct serial
   {
     int fd;			/* File descriptor */
+    int error_fd;               /* File descriptor for a separate
+				   error stream that should be
+				   immediately forwarded to gdb_stderr.
+				   This may be -1.  
+				   If != -1, this descriptor should
+				   be non-blocking.  */
     struct serial_ops *ops;	/* Function vector */
     void *state;       		/* Local context info for open FD */
     serial_ttystate ttystate;	/* Not used (yet) */
--- gdb/ser-pipe.c	(/mirrors/gdb_mainline)	(revision 3417)
+++ gdb/ser-pipe.c	(/patches/gdb/remote_stderr_try2/gdb_mainline)	(revision 3417)
@@ -62,9 +62,12 @@ pipe_open (struct serial *scb, const cha
    * published in UNIX Review, Vol. 6, No. 8.
    */
   int pdes[2];
+  int err_pdes[2];
   int pid;
   if (socketpair (AF_UNIX, SOCK_STREAM, 0, pdes) < 0)
     return -1;
+  if (socketpair (AF_UNIX, SOCK_STREAM, 0, err_pdes) < 0)
+    return -1;
 
   /* Create the child process to run the command in.  Note that the
      apparent call to vfork() below *might* actually be a call to
@@ -77,9 +80,18 @@ pipe_open (struct serial *scb, const cha
     {
       close (pdes[0]);
       close (pdes[1]);
+      close (err_pdes[0]);
+      close (err_pdes[1]);
       return -1;
     }
 
+  if (fcntl (err_pdes[0], F_SETFL, O_NONBLOCK) == -1)
+    {
+      close (err_pdes[0]);
+      close (err_pdes[1]);
+      err_pdes[0] = err_pdes[1] = -1;
+    }
+
   /* Child. */
   if (pid == 0)
     {
@@ -91,6 +103,13 @@ pipe_open (struct serial *scb, const cha
 	  close (pdes[1]);
 	}
       dup2 (STDOUT_FILENO, STDIN_FILENO);
+
+      if (err_pdes[0] != -1)
+	{
+	  close (err_pdes[0]);
+	  dup2 (err_pdes[1], STDERR_FILENO);
+	  close (err_pdes[1]);
+	}
 #if 0
       /* close any stray FD's - FIXME - how? */
       /* POSIX.2 B.3.2.2 "popen() shall ensure that any streams
@@ -109,6 +128,7 @@ pipe_open (struct serial *scb, const cha
   state = XMALLOC (struct pipe_state);
   state->pid = pid;
   scb->fd = pdes[0];
+  scb->error_fd = err_pdes[0];
   scb->state = state;
 
   /* If we don't do this, GDB simply exits when the remote side dies.  */
--- gdb/ser-base.c	(/mirrors/gdb_mainline)	(revision 3417)
+++ gdb/ser-base.c	(/patches/gdb/remote_stderr_try2/gdb_mainline)	(revision 3417)
@@ -314,6 +314,7 @@ generic_readchar (struct serial *scb, in
 		  int (do_readchar) (struct serial *scb, int timeout))
 {
   int ch;
+
   if (scb->bufcnt > 0)
     {
       ch = *scb->bufp;
@@ -343,6 +344,51 @@ generic_readchar (struct serial *scb, in
 	    }
 	}
     }
+  /* Read any error output we might have.  */
+  if (scb->error_fd != -1)
+    {
+      ssize_t s;
+      char buf[81];
+
+      for (;;)
+        {
+ 	  char *current;
+ 	  char *newline;
+
+#ifdef _WIN32
+	  /* On windows, it's not easy to make a pipe non-blocking,
+	     if at all possible. Therefore, first find out
+	     how many bytes are in the pipes and read only the available
+	     number of bytes so that to avoid blocking.  */
+	  HANDLE h = (HANDLE) _get_osfhandle (scb->error_fd);
+	  DWORD numBytes;
+	  BOOL r = PeekNamedPipe (h, NULL, 0, NULL, &numBytes, NULL);
+	  if (r == FALSE || numBytes == 0)
+	    break;
+	  s = read (scb->error_fd, &buf, numBytes > 80 ? 80 : numBytes);
+#else
+	  s = read (scb->error_fd, &buf, 80);
+#endif	  
+	  if (s == -1)
+	    break;
+
+	  /* In theory, embedded newlines are not a problem.
+	     But for MI, we want each output line to have just
+	     one newline for legibility.  So output things
+	     in newline chunks.  */
+	  buf[s] = '\0';
+	  current = buf;
+	  while ((newline = strstr (current, "\n")) != NULL)
+	    {
+	      *newline = '\0';
+	      fputs_unfiltered (current, gdb_stderr);
+	      fputs_unfiltered ("\n", gdb_stderr);
+	      current = newline + 1;
+	    }
+	  fputs_unfiltered (current, gdb_stderr);
+	}
+    }
+
   reschedule (scb);
   return ch;
 }
--- gdb/ser-mingw.c	(/mirrors/gdb_mainline)	(revision 3417)
+++ gdb/ser-mingw.c	(/patches/gdb/remote_stderr_try2/gdb_mainline)	(revision 3417)
@@ -697,6 +697,7 @@ static int
 pipe_windows_open (struct serial *scb, const char *name)
 {
   struct pipe_state *ps;
+  FILE *pex_stderr;
 
   char **argv = buildargv (name);
   struct cleanup *back_to = make_cleanup_freeargv (argv);
@@ -717,7 +718,8 @@ pipe_windows_open (struct serial *scb, c
   {
     int err;
     const char *err_msg
-      = pex_run (ps->pex, PEX_SEARCH | PEX_BINARY_INPUT | PEX_BINARY_OUTPUT,
+      = pex_run (ps->pex, PEX_SEARCH | PEX_BINARY_INPUT | PEX_BINARY_OUTPUT
+		 | PEX_STDERR_TO_PIPE,
                  argv[0], argv, NULL, NULL,
                  &err);
 
@@ -739,8 +741,13 @@ pipe_windows_open (struct serial *scb, c
   ps->output = pex_read_output (ps->pex, 1);
   if (! ps->output)
     goto fail;
-
   scb->fd = fileno (ps->output);
+
+  pex_stderr = pex_read_err (ps->pex, 1);
+  if (! pex_stderr)
+    goto fail;
+  scb->error_fd = fileno (pex_stderr);
+
   scb->state = (void *) ps;
 
   discard_cleanups (back_to);

Property changes on: 
___________________________________________________________________
Name: csl:base
 +/all/mirrors/gdb_mainline
Name: svk:merge
 +e7755896-6108-0410-9592-8049d3e74e28:/mirrors/gdb/trunk:162899