generic `struct serial' interface pipe for remote non-stop

[Pedro Alves <pedro at codesourcery dot com>]

Hi,

(Some ramp-up first.)

Non-stop mode works on top of the asynchronous mode, because in non-stop mode,
you naturally want to be able to issue commands while the inferior is running.

One requirement of the asynchronous mode, is that instead of blocking in
target_wait waiting for the inferior to stop, the target registers a
waitable file descriptor in the event loop, that is written to whenever
the target has something to report to the core (say, a SIGTRAP).  This way,
the event loop continues running and handling CLI/MI commands while the
inferior is running.

The remote.c target registers the serial descriptor (actually a
`struct serial' interface) that is used to communicate with the remote
server to that effect.  This way, whenever there is communication going
in the server -> GDB direction, the event loop calls the associated
callback which ends up calling target_wait -> remote_wait to collect
the event the stub is reporting, and the immediatelly returns again to
the core, handling the event, and going back to the event loop.

For non-stop mode, however, I'll need two other event sources registered in
the event loop, that are independant of the remote communication, as you'll
see in the patch I'll post soon.  The easiest way to get us a select'able
file descriptor with the properties we want, is a pipe.  We register the
read end in the event loop, and write something to the write end whenever
we have something interesting to tell the core about.  This is what I've
done in the linux native implementation (linux-nat.c).

Now, the problem with remote.c, is that it is built on all hosts, and it
happens that on Windows, the file descriptors returned from `_pipe'
are not waitable on.  The handles behind them belongs to an anonymous pipe,
and WaitForMultipleObjects doesn't work on those.

Now, we already have infrastructure in place that abstracts a bunch
or serial interface types (serials, pipes, tcp, udp, etc.).  And
"target remote | " does work on Windows.  So, I though that the
best practical solution for this would be to reuse it.

That's what the attached patch does.

Without going much over the details of the remote non-stop
implementation, here's a snippet of how I intend to use it in
remote.c:

 /* Pipes for use by the asynchronous event processing used in non-stop
    mode.  */

 static struct serial *remote_inferior_event_pipe[2];
 static struct serial *remote_get_pending_events_pipe[2];

 static void
 make_event_pipes (void)
 {
   /* Create event pipes and register them in the event loop.  */
   if (serial_pipe (remote_inferior_event_pipe) == -1
       || serial_pipe (remote_get_pending_events_pipe) == -1)
     perror_with_name(_("Creating non-stop event files failed"));

   /* Register the read handles in the event loop.  */
   serial_async (remote_inferior_event_pipe[0],
 		remote_async_inferior_event_handler, NULL);
   serial_async (remote_get_pending_events_pipe[0],
 		remote_async_get_pending_events_handler, NULL);
 }

Calling serial_open to open a pipe doesn't cut it, because that is
designed to spawn a child process and communicate with it through
pipes.  I don't want to spawn a new process, and I want both ends
of the pipe.  To that end, I've added a new function `gdb_pipe' to
ser-go32.c, ser-ming32.c and ser-pipe.c, which should satisfy the
linking on all possible hosts:

configure.ac:
 dnl Figure out which of the many generic ser-*.c files the _host_ supports.
 SER_HARDWIRE="ser-base.o ser-unix.o ser-pipe.o ser-tcp.o"
 case ${host} in
   *go32* ) SER_HARDWIRE=ser-go32.o ;;
   *djgpp* ) SER_HARDWIRE=ser-go32.o ;;
   *mingw32*) SER_HARDWIRE="ser-base.o ser-tcp.o ser-mingw.o" ;;
 esac
 AC_SUBST(SER_HARDWIRE)

For go32/djgpp or for unix hosts that don't have socketpair, I
just bail out with ENOSYS.

I few other tweaks were needed, because I needed a way
for serial_fdopen to call into the serial_ops implementation to
do some initialisations.  Anyway, the end result is a new
shiny function called:

 int serial_pipe (struct serial *scbs[2]);

That the remote.c code can use, so it can stay agnostic of
which host is building it.

Anyone see a better way to do this?  Does it sound reasonable?

-- 
Pedro Alves

2008-10-16  Pedro Alves  <pedro@codesourcery.com>

	* serial.h (gdb_pipe, serial_pipe): Declare.
	* serial.c (serial_interface_lookup): Take a const char pointer.
	(serial_fdopen): Rename to ...
	(serial_fdopen_ops): ... this.  Add an OPS parameter and use it.
	Call the OPS' fdopen function if there is one.
	(serial_fdopen): Rewrite as wrapper to serial_fdopen_ops.
	(serial_pipe): New.
	(struct serial_ops) <fdopen>: New field.

	* ser-mingw.c (free_pipe_state):
	(free_pipe_state): Close output on non-pex pipes.
	(pipe_windows_fdopen): New.
	(gdb_pipe): New.
	(_initialize_ser_windows): Register pipe_windows_fdopen.
	* ser-go32.c (gdb_pipe): New.
	* ser-pipe.c (pipe_close): Close file descriptor even if there's
	no state pointer.
	(pipe_ops): Delete.
	(gdb_pipe): New.

---
 gdb/ser-go32.c  |    7 ++++++
 gdb/ser-mingw.c |   41 +++++++++++++++++++++++++++++++++--
 gdb/ser-pipe.c  |   29 +++++++++++++++++++-----
 gdb/serial.c    |   65 +++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/serial.h    |   14 ++++++++++++
 5 files changed, 133 insertions(+), 23 deletions(-)

Index: src/gdb/serial.h
===================================================================
--- src.orig/gdb/serial.h	2008-10-16 18:23:31.000000000 +0100
+++ src/gdb/serial.h	2008-10-17 00:02:51.000000000 +0100
@@ -55,6 +55,19 @@ extern struct serial *serial_fdopen (con
 
 extern void serial_close (struct serial *scb);
 
+/* Create a pipe, and put the read end in files[0], and the write end
+   in filde[1].  Returns 0 for success, negative value for error (in
+   which case errno contains the error).  */
+
+extern int gdb_pipe (int fildes[2]);
+
+/* Create a pipe with each end wrapped in a `struct serial' interface.
+   Put the read end in scbs[0], and the write end in scbs[1].  Returns
+   0 for success, negative value for error (in which case errno
+   contains the error).  */
+
+extern int serial_pipe (struct serial *scbs[2]);
+
 /* Push out all buffers and destroy SCB without closing the device.  */
 
 extern void serial_un_fdopen (struct serial *scb);
@@ -221,6 +234,7 @@ struct serial_ops
     struct serial_ops *next;
     int (*open) (struct serial *, const char *name);
     void (*close) (struct serial *);
+    int (*fdopen) (struct serial *, int fd);
     int (*readchar) (struct serial *, int timeout);
     int (*write) (struct serial *, const char *str, int len);
     /* Discard pending output */
Index: src/gdb/serial.c
===================================================================
--- src.orig/gdb/serial.c	2008-10-16 18:23:31.000000000 +0100
+++ src/gdb/serial.c	2008-10-16 18:23:44.000000000 +0100
@@ -48,7 +48,7 @@ static struct serial *scb_base;
 static char *serial_logfile = NULL;
 static struct ui_file *serial_logfp = NULL;
 
-static struct serial_ops *serial_interface_lookup (char *);
+static struct serial_ops *serial_interface_lookup (const char *);
 static void serial_logchar (struct ui_file *stream, int ch_type, int ch, int timeout);
 static const char logbase_hex[] = "hex";
 static const char logbase_octal[] = "octal";
@@ -146,7 +146,7 @@ serial_log_command (const char *cmd)
 
 
 static struct serial_ops *
-serial_interface_lookup (char *name)
+serial_interface_lookup (const char *name)
 {
   struct serial_ops *ops;
 
@@ -255,22 +255,27 @@ serial_for_fd (int fd)
   return NULL;
 }
 
-struct serial *
-serial_fdopen (const int fd)
+/* Open a new serial stream using a file handle, using serial
+   interface ops OPS.  */
+
+static struct serial *
+serial_fdopen_ops (const int fd, struct serial_ops *ops)
 {
   struct serial *scb;
-  struct serial_ops *ops;
 
-  for (scb = scb_base; scb; scb = scb->next)
-    if (scb->fd == fd)
-      {
-	scb->refcnt++;
-	return scb;
-      }
+  scb = serial_for_fd (fd);
+  if (scb)
+    {
+      scb->refcnt++;
+      return scb;
+    }
 
-  ops = serial_interface_lookup ("terminal");
   if (!ops)
-    ops = serial_interface_lookup ("hardwire");
+    {
+      ops = serial_interface_lookup ("terminal");
+      if (!ops)
+ 	ops = serial_interface_lookup ("hardwire");
+    }
 
   if (!ops)
     return NULL;
@@ -282,8 +287,6 @@ serial_fdopen (const int fd)
   scb->bufcnt = 0;
   scb->bufp = scb->buf;
 
-  scb->fd = fd;
-
   scb->name = NULL;
   scb->next = scb_base;
   scb->refcnt = 1;
@@ -293,11 +296,22 @@ serial_fdopen (const int fd)
   scb->async_context = NULL;
   scb_base = scb;
 
+  if ((ops->fdopen) != NULL)
+    (*ops->fdopen) (scb, fd);
+  else
+    scb->fd = fd;
+
   last_serial_opened = scb;
 
   return scb;
 }
 
+struct serial *
+serial_fdopen (const int fd)
+{
+  return serial_fdopen_ops (fd, NULL);
+}
+
 static void
 do_serial_close (struct serial *scb, int really_close)
 {
@@ -569,6 +583,27 @@ serial_done_wait_handle (struct serial *
 }
 #endif
 
+int
+serial_pipe (struct serial *scbs[2])
+{
+  struct serial_ops *ops;
+  int fildes[2];
+
+  ops = serial_interface_lookup ("pipe");
+  if (!ops)
+    {
+      errno = ENOSYS;
+      return -1;
+    }
+
+  if (gdb_pipe (fildes) == -1)
+    return -1;
+
+  scbs[0] = serial_fdopen_ops (fildes[0], ops);
+  scbs[1] = serial_fdopen_ops (fildes[1], ops);
+  return 0;
+}
+
 #if 0
 /* The connect command is #if 0 because I hadn't thought of an elegant
    way to wait for I/O on two `struct serial *'s simultaneously.  Two
Index: src/gdb/ser-mingw.c
===================================================================
--- src.orig/gdb/ser-mingw.c	2008-10-16 18:23:31.000000000 +0100
+++ src/gdb/ser-mingw.c	2008-10-16 18:23:44.000000000 +0100
@@ -798,8 +798,12 @@ free_pipe_state (struct pipe_state *ps)
   if (ps->input)
     fclose (ps->input);
   if (ps->pex)
-    pex_free (ps->pex);
-  /* pex_free closes ps->output.  */
+    {
+      pex_free (ps->pex);
+      /* pex_free closes ps->output.  */
+    }
+  else if (ps->output)
+    fclose (ps->output);
 
   xfree (ps);
 
@@ -884,6 +888,30 @@ pipe_windows_open (struct serial *scb, c
   return -1;
 }
 
+static int
+pipe_windows_fdopen (struct serial *scb, int fd)
+{
+  struct pipe_state *ps;
+
+  ps = make_pipe_state ();
+
+  ps->input = fdopen (fd, "r+");
+  if (! ps->input)
+    goto fail;
+
+  ps->output = fdopen (fd, "r+");
+  if (! ps->output)
+    goto fail;
+
+  scb->fd = fd;
+  scb->state = (void *) ps;
+
+  return 0;
+
+ fail:
+  free_pipe_state (ps);
+  return -1;
+}
 
 static void
 pipe_windows_close (struct serial *scb)
@@ -987,6 +1015,14 @@ pipe_avail (struct serial *scb, int fd)
   return numBytes;
 }
 
+int
+gdb_pipe (int pdes[2])
+{
+  if (_pipe (pdes, 512, _O_BINARY | _O_NOINHERIT) == -1)
+    return -1;
+  return 0;
+}
+
 struct net_windows_state
 {
   struct ser_console_state base;
@@ -1225,6 +1261,7 @@ _initialize_ser_windows (void)
   ops->next = 0;
   ops->open = pipe_windows_open;
   ops->close = pipe_windows_close;
+  ops->fdopen = pipe_windows_fdopen;
   ops->readchar = ser_base_readchar;
   ops->write = ser_base_write;
   ops->flush_output = ser_base_flush_output;
Index: src/gdb/ser-go32.c
===================================================================
--- src.orig/gdb/ser-go32.c	2008-10-16 18:23:31.000000000 +0100
+++ src/gdb/ser-go32.c	2008-10-16 18:23:44.000000000 +0100
@@ -858,6 +858,13 @@ static struct serial_ops dos_ops =
   (void (*)(struct serial *, int))NULL	/* change into async mode */
 };
 
+int
+gdb_pipe (int pdes[2])
+{
+  /* No support for pipes.  */
+  errno = ENOSYS;
+  return -1;
+}
 
 static void
 dos_info (char *arg, int from_tty)
Index: src/gdb/ser-pipe.c
===================================================================
--- src.orig/gdb/ser-pipe.c	2008-10-16 18:23:31.000000000 +0100
+++ src/gdb/ser-pipe.c	2008-10-16 22:13:39.000000000 +0100
@@ -139,19 +139,36 @@ static void
 pipe_close (struct serial *scb)
 {
   struct pipe_state *state = scb->state;
+
+  close (scb->fd);
+  scb->fd = -1;
+
   if (state != NULL)
     {
-      int pid = state->pid;
-      close (scb->fd);
-      scb->fd = -1;
+      kill (state->pid, SIGTERM);
+      /* Might be useful to check that the child does die.  */
+
       xfree (state);
       scb->state = NULL;
-      kill (pid, SIGTERM);
-      /* Might be useful to check that the child does die. */
     }
 }
 
-static struct serial_ops pipe_ops;
+int
+gdb_pipe (int pdes[2])
+{
+#if !HAVE_SOCKETPAIR
+  errno = ENOSYS;
+  return -1;
+#else
+
+  if (socketpair (AF_UNIX, SOCK_STREAM, 0, pdes) < 0)
+    return -1;
+
+  /* If we don't do this, GDB simply exits when the remote side dies.  */
+  signal (SIGPIPE, SIG_IGN);
+  return 0;
+#endif
+}
 
 void
 _initialize_ser_pipe (void)