Re: [patch] Implement post_event for Python scripts.

[Pedro Alves <pedro at codesourcery dot com>]

On Wednesday 18 August 2010 15:36:11, Daniel Jacobowitz wrote:
> On Wed, Aug 18, 2010 at 02:45:52PM +0100, Pedro Alves wrote:
> > I'm also not sure whether ...
> > 
> > > /* We use a file handler, and not an async handler, so that we can
> > >    wake up the main thread even when it is blocked in poll().  */
> > > static int gdbpy_event_fds[2];
> > 
> > ... mingw-htep.c:gdb_select will be smart enough to apply the
> > proper select for that file descriptor, or if we need to bring
> > the whole patch in.
> 
> I think it'll work; it falls back to _get_osfhandle, So if you can
> WaitForMultipleObjects on the Windows pipe handle, it'll do OK.

If I am to trust my 2008-self, if we simply fallback like that, it won't
work.  The file descriptors returned from `_pipe' are not waitable
on.  The handles behind them belong to an anonymous pipe, and
WaitForMultipleObjects doesn't work on those.  I'm just starting
to look into this, so I guess I'll know better once I actually
try it out.

> It was just for some serial cases where you needed a separate handle that
> we had problems.  

> OTOH if waiting for the pipe handle means you can
> connect to it, or some other interpretation, we'll be back to the
> beginning.  

Not sure I understood.

> You could maybe adapt ser-pipe.

If I'm understanding you here, that's what the patch I pointed at
did.  Here's the same thing refreshed to head.  The idea of the patch
originally was to use "serial_pipe" (working at the struct serial layer)
instead of gdb_pipe in common code.

I'll know more in a bit.  I have to relearn Windows.  :-)

-- 
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  |   33 ++++++++++++++++++++++------
 gdb/serial.c    |   65 +++++++++++++++++++++++++++++++++++++++++++-------------
 gdb/serial.h    |   14 ++++++++++++
 5 files changed, 136 insertions(+), 24 deletions(-)

Index: src/gdb/serial.h
===================================================================
--- src.orig/gdb/serial.h	2010-06-16 10:58:26.000000000 +0100
+++ src/gdb/serial.h	2010-08-18 18:22:01.000000000 +0100
@@ -56,6 +56,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);
@@ -222,6 +235,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	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/serial.c	2010-08-18 18:22:01.000000000 +0100
@@ -49,7 +49,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";
@@ -147,7 +147,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)
 {
@@ -582,6 +596,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	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/ser-mingw.c	2010-08-18 18:22:01.000000000 +0100
@@ -802,8 +802,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);
 
@@ -888,6 +892,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)
@@ -992,6 +1020,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;
@@ -1230,6 +1266,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	2010-06-16 11:15:34.000000000 +0100
+++ src/gdb/ser-go32.c	2010-08-18 18:22:01.000000000 +0100
@@ -859,6 +859,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	2010-06-16 12:36:45.000000000 +0100
+++ src/gdb/ser-pipe.c	2010-08-18 18:24:02.000000000 +0100
@@ -157,23 +157,42 @@ 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,
+	 and while we're waiting for it to die print any remaining
+	 stderr output.  */
+
       if (scb->error_fd != -1)
 	close (scb->error_fd);
       scb->error_fd = -1;
       xfree (state);
       scb->state = NULL;
-      kill (pid, SIGTERM);
-      /* Might be useful to check that the child does die,
-	 and while we're waiting for it to die print any remaining
-	 stderr output.  */
     }
 }
 
+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)
 {