PATCH: Support Windows in event-loop.c

PATCH: Support Windows in event-loop.c

[Mark Mitchell]

This is the last wpatch required to GDB proper to support execution on
Windows; the only other patches are relatively minor changes to
readline.

This patch adds support for Windows to event-loop.c.  The key issue is
that "select" on Windows only works on sockets; it does not work on
general file descriptors.  For that, one must use
WaitForMultipleObjects, and that requires converting file descriptors
(as used by read/write/open/close) to native HANDLEs. 

OK to commit?

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com

2005-03-28  Mark Mitchell  <mark@codesourcery.com>

	* gdb/event-loop.c (struct gdb_notifier): Add "handles" for Windows.
	(create_file_handler): On Windows, update handles, rather than
	check_masks and ready_masks.  
	(delete_file_handler): Likewise.
	(gdb_wait_for_event): Use WaitForMultipleObjects, not select, on
	Windows.

Index: event-loop.c
===================================================================
RCS file: /cvs/src/src/gdb/event-loop.c,v
retrieving revision 1.24
diff -c -5 -p -r1.24 event-loop.c
*** event-loop.c	12 Feb 2005 00:39:18 -0000	1.24
--- event-loop.c	21 Apr 2005 05:42:47 -0000
*************** event_queue;
*** 131,140 ****
--- 131,145 ----
  #define USE_POLL 0
  #endif /* HAVE_POLL */
  
  static unsigned char use_poll = USE_POLL;
  
+ #ifdef USE_WIN32API
+ #include <windows.h>
+ #include <io.h>
+ #endif
+ 
  static struct
    {
      /* Ptr to head of file handler list. */
      file_handler *first_file_handler;
  
*************** static struct
*** 144,162 ****
  
      /* Timeout in milliseconds for calls to poll(). */
      int poll_timeout;
  #endif
  
      /* Masks to be used in the next call to select.
         Bits are set in response to calls to create_file_handler. */
      fd_set check_masks[3];
  
      /* What file descriptors were found ready by select. */
      fd_set ready_masks[3];
! 
      /* Number of file descriptors to monitor. (for poll) */
      /* Number of valid bits (highest fd value + 1). (for select) */
      int num_fds;
  
      /* Time structure for calls to select(). */
      struct timeval select_timeout;
  
--- 149,171 ----
  
      /* Timeout in milliseconds for calls to poll(). */
      int poll_timeout;
  #endif
  
+ #ifdef USE_WIN32API
+     HANDLE handles[MAXIMUM_WAIT_OBJECTS];
+ #else
      /* Masks to be used in the next call to select.
         Bits are set in response to calls to create_file_handler. */
      fd_set check_masks[3];
  
      /* What file descriptors were found ready by select. */
      fd_set ready_masks[3];
! #endif
      /* Number of file descriptors to monitor. (for poll) */
      /* Number of valid bits (highest fd value + 1). (for select) */
+     /* Number of handles (for Windows).  */
      int num_fds;
  
      /* Time structure for calls to select(). */
      struct timeval select_timeout;
  
*************** create_file_handler (int fd, int mask, h
*** 522,531 ****
--- 531,544 ----
  			  _("use_poll without HAVE_POLL"));
  #endif /* HAVE_POLL */
  	}
        else
  	{
+ #ifdef USE_WIN32API
+ 	  gdb_notifier.handles[gdb_notifier.num_fds++] 
+ 	    = (HANDLE) _get_osfhandle (fd);
+ #else
  	  if (mask & GDB_READABLE)
  	    FD_SET (fd, &gdb_notifier.check_masks[0]);
  	  else
  	    FD_CLR (fd, &gdb_notifier.check_masks[0]);
  
*************** create_file_handler (int fd, int mask, h
*** 539,548 ****
--- 552,562 ----
  	  else
  	    FD_CLR (fd, &gdb_notifier.check_masks[2]);
  
  	  if (gdb_notifier.num_fds <= fd)
  	    gdb_notifier.num_fds = fd + 1;
+ #endif
  	}
      }
  
    file_ptr->proc = proc;
    file_ptr->client_data = client_data;
*************** delete_file_handler (int fd)
*** 600,609 ****
--- 614,633 ----
  		      _("use_poll without HAVE_POLL"));
  #endif /* HAVE_POLL */
      }
    else
      {
+ #ifdef USE_WIN32API
+       HANDLE h = (HANDLE) _get_osfhandle (fd);
+       for (i = 0; i < gdb_notifier.num_fds; ++i)
+ 	if (gdb_notifier.handles[i] == h)
+ 	  {
+ 	    gdb_notifier.handles[i] =
+ 	      gdb_notifier.handles[gdb_notifier.num_fds--];
+ 	    break;
+ 	  }
+ #else
        if (file_ptr->mask & GDB_READABLE)
  	FD_CLR (fd, &gdb_notifier.check_masks[0]);
        if (file_ptr->mask & GDB_WRITABLE)
  	FD_CLR (fd, &gdb_notifier.check_masks[1]);
        if (file_ptr->mask & GDB_EXCEPTION)
*************** delete_file_handler (int fd)
*** 621,630 ****
--- 645,655 ----
  		  || FD_ISSET (i - 1, &gdb_notifier.check_masks[2]))
  		break;
  	    }
  	  gdb_notifier.num_fds = i;
  	}
+ #endif
      }
  
    /* Deactivate the file descriptor, by clearing its mask, 
       so that it will not fire again. */
  
*************** handle_file_event (int event_file_desc)
*** 735,745 ****
--- 760,774 ----
  static int
  gdb_wait_for_event (void)
  {
    file_handler *file_ptr;
    gdb_event *file_event_ptr;
+ #ifdef USE_WIN32API
+   DWORD event = 0;
+ #else
    int num_found = 0;
+ #endif
    int i;
  
    /* Make sure all output is done before getting another event. */
    gdb_flush (gdb_stdout);
    gdb_flush (gdb_stderr);
*************** gdb_wait_for_event (void)
*** 764,773 ****
--- 793,812 ----
  		      _("use_poll without HAVE_POLL"));
  #endif /* HAVE_POLL */
      }
    else
      {
+ #ifdef USE_WIN32API
+       event
+ 	= WaitForMultipleObjects(gdb_notifier.num_fds,
+ 				 gdb_notifier.handles,
+ 				 FALSE,
+ 				 gdb_notifier.timeout_valid
+ 				 ? (gdb_notifier.select_timeout.tv_sec * 1000
+ 				    + gdb_notifier.select_timeout.tv_usec)
+ 				 : INFINITE);
+ #else
        gdb_notifier.ready_masks[0] = gdb_notifier.check_masks[0];
        gdb_notifier.ready_masks[1] = gdb_notifier.check_masks[1];
        gdb_notifier.ready_masks[2] = gdb_notifier.check_masks[2];
        num_found = select (gdb_notifier.num_fds,
  			  &gdb_notifier.ready_masks[0],
*************** gdb_wait_for_event (void)
*** 784,793 ****
--- 823,833 ----
  	  FD_ZERO (&gdb_notifier.ready_masks[2]);
  	  /* Dont print anything is we got a signal, let gdb handle it. */
  	  if (errno != EINTR)
  	    perror_with_name (("select"));
  	}
+ #endif
      }
  
    /* Enqueue all detected file events. */
  
    if (use_poll)
*************** gdb_wait_for_event (void)
*** 826,835 ****
--- 866,889 ----
  		      _("use_poll without HAVE_POLL"));
  #endif /* HAVE_POLL */
      }
    else
      {
+ #ifdef USE_WIN32API
+       HANDLE h;
+ 
+       h = gdb_notifier.handles[event - WAIT_OBJECT_0];
+       file_ptr = gdb_notifier.first_file_handler;
+       while ((HANDLE) _get_osfhandle (file_ptr->fd) != h)
+ 	file_ptr = file_ptr->next_file;
+       if (file_ptr->ready_mask == 0)
+ 	{
+ 	  file_event_ptr = create_file_event (file_ptr->fd);
+ 	  async_queue_event (file_event_ptr, TAIL);
+ 	}
+       file_ptr->ready_mask = GDB_READABLE;
+ #else      
        for (file_ptr = gdb_notifier.first_file_handler;
  	   (file_ptr != NULL) && (num_found > 0);
  	   file_ptr = file_ptr->next_file)
  	{
  	  int mask = 0;
*************** gdb_wait_for_event (void)
*** 854,863 ****
--- 908,918 ----
  	      file_event_ptr = create_file_event (file_ptr->fd);
  	      async_queue_event (file_event_ptr, TAIL);
  	    }
  	  file_ptr->ready_mask = mask;
  	}
+ #endif
      }
    return 0;
  }
  \f
  

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Wed, 20 Apr 2005 22:49:02 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> 
> This patch adds support for Windows to event-loop.c.  The key issue is
> that "select" on Windows only works on sockets; it does not work on
> general file descriptors.  For that, one must use
> WaitForMultipleObjects, and that requires converting file descriptors
> (as used by read/write/open/close) to native HANDLEs. 

Ouch! ugly OS-dependent #ifdef's in event-loop.c!

Is it perhaps possible to write an emulation of `select' that would
handle file handles as well, put it on win32-nat.c, say, and then
leave event-loop.c more or less alone?  That would be much cleaner, I
think.

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Thu, Apr 21, 2005 at 09:41:28PM +0300, Eli Zaretskii wrote:
> > Date: Wed, 20 Apr 2005 22:49:02 -0700
> > From: Mark Mitchell <mark@codesourcery.com>
> > 
> > This patch adds support for Windows to event-loop.c.  The key issue is
> > that "select" on Windows only works on sockets; it does not work on
> > general file descriptors.  For that, one must use
> > WaitForMultipleObjects, and that requires converting file descriptors
> > (as used by read/write/open/close) to native HANDLEs. 
> 
> Ouch! ugly OS-dependent #ifdef's in event-loop.c!
> 
> Is it perhaps possible to write an emulation of `select' that would
> handle file handles as well, put it on win32-nat.c, say, and then
> leave event-loop.c more or less alone?  That would be much cleaner, I
> think.

If so, please don't use win32-nat.c; this is Windows hosting support
rather than Windows native debugging support.

Honestly, I don't find Mark's changes any worse than the already
present ifdefs to select between select and poll.  It's the same sort
of choice.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Eli Zaretskii wrote:

> Is it perhaps possible to write an emulation of `select' that would
> handle file handles as well

Well, Cygwin has select, so it is *possible*.  But, it's not easy, and 
it doesn't really map terribly well onto what Windows provides.

As Daniel says, this is very much analogous to poll/select; different 
systems provide different low-level mechanisms for waiting for input.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Thu, 21 Apr 2005 11:56:02 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> CC: gdb-patches@sources.redhat.com
> 
> Eli Zaretskii wrote:
> 
> > Is it perhaps possible to write an emulation of `select' that would
> > handle file handles as well
> 
> Well, Cygwin has select, so it is *possible*.  But, it's not easy, and 
> it doesn't really map terribly well onto what Windows provides.

Perhaps you could look at w32proc.c:sys_select in the Emacs
distribution, and copy its code with minimal changes.

> As Daniel says, this is very much analogous to poll/select; different 
> systems provide different low-level mechanisms for waiting for input.

It isn't analogous: HAVE_POLL tests for a system-independent
functionality, not unlike HAVE_UNISTD_H, while USE_WIN32API tests for
an OS-specific misfeature and names an OS-specific macro.

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Thu, Apr 21, 2005 at 11:27:59PM +0300, Eli Zaretskii wrote:
> > Date: Thu, 21 Apr 2005 11:56:02 -0700
> > From: Mark Mitchell <mark@codesourcery.com>
> > CC: gdb-patches@sources.redhat.com
> > 
> > Eli Zaretskii wrote:
> > 
> > > Is it perhaps possible to write an emulation of `select' that would
> > > handle file handles as well
> > 
> > Well, Cygwin has select, so it is *possible*.  But, it's not easy, and 
> > it doesn't really map terribly well onto what Windows provides.
> 
> Perhaps you could look at w32proc.c:sys_select in the Emacs
> distribution, and copy its code with minimal changes.
> 
> > As Daniel says, this is very much analogous to poll/select; different 
> > systems provide different low-level mechanisms for waiting for input.
> 
> It isn't analogous: HAVE_POLL tests for a system-independent
> functionality, not unlike HAVE_UNISTD_H, while USE_WIN32API tests for
> an OS-specific misfeature and names an OS-specific macro.

I don't follow this objection:

  - I don't think that we get to call OS-specific interfaces
    "misfeatures".

  - I don't think that answering your objection with an autoconf test for
    WaitForMultipleObjects would make you any happier.  Would it?

How does the fact that only Windows provides WaitForMultipleObjects
make it conceptually different from the fact that a bunch of systems
provide poll, and a bunch don't?

Anyway, if Mark can come up with a select wrapper, then maybe we can
drop the question entirely.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Thu, Apr 21, 2005 at 11:56:02AM -0700, Mark Mitchell wrote:
>Eli Zaretskii wrote:
>>Is it perhaps possible to write an emulation of `select' that would
>>handle file handles as well
>
>Well, Cygwin has select, so it is *possible*.  But, it's not easy, and
>it doesn't really map terribly well onto what Windows provides.

cygwin's select is pretty complicated but most of that is due to thread
safety considerations.  If there are only a limited number of types of
devices being selected, it should be possible to mock something up
without only marginal pain.

What kind of things are being "selected"?  I'm surprised that
WaitForMultipleObjects would be all that useful here since, except in
the case of console I/O, it only really is used for event (e.g., events,
semaphores, mutexes, thread termination) types of operations.  It
wouldn't be useful for pipes, for instance.  And it wouldn't be useful
for serial I/O unless the serial device was opened on overlapped mode.

cgf

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Thu, Apr 21, 2005 at 05:01:49PM -0400, Christopher Faylor wrote:
>On Thu, Apr 21, 2005 at 11:56:02AM -0700, Mark Mitchell wrote:
>>Eli Zaretskii wrote:
>>>Is it perhaps possible to write an emulation of `select' that would
>>>handle file handles as well
>>
>>Well, Cygwin has select, so it is *possible*.  But, it's not easy, and
>>it doesn't really map terribly well onto what Windows provides.
>
>cygwin's select is pretty complicated but most of that is due to thread
>safety considerations.  If there are only a limited number of types of
>devices being selected, it should be possible to mock something up
>without only marginal pain.

Oops.  Duh.  Make that "with only marginal pain".

cgf

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Daniel Jacobowitz wrote:

> Anyway, if Mark can come up with a select wrapper, then maybe we can
> drop the question entirely.

I'm sure I can, if that's the only way to get this functionality into 
GDB.  But, the emacs select wrapper is hardly a full implementation of 
select; for example, it only handles file descriptors waiting for 
"read", not "write".  To me, adding a partial emulation of a POSIX 
function, without real POSIX semantics and behavior, is confusing.  And 
adding a full emulation is going to be a lot more code than the present 
change -- and will perform worse to boot.

Windows isn't UNIX, and waiting for activity on file descriptors is one 
of the places where Windows made some rather different choices.  This is 
a low-level system-programming code, and so system details appear.  It's 
like dealing with different rules for interruptable syscalls, different 
signal-handling rules, and so forth.

Fortunately, it doesn't look like event-loop.c is a very active area of 
development.  It looks like the last substantive change was on 
2002-05-14.  It seems unlikely to me that this change is going to make 
it a lot harder for people to make other improvements to event-loop.c.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Thu, 21 Apr 2005 16:56:17 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Mark Mitchell <mark@codesourcery.com>, gdb-patches@sources.redhat.com
> 
> > It isn't analogous: HAVE_POLL tests for a system-independent
> > functionality, not unlike HAVE_UNISTD_H, while USE_WIN32API tests for
> > an OS-specific misfeature and names an OS-specific macro.
> 
> I don't follow this objection:
> 
>   - I don't think that we get to call OS-specific interfaces
>     "misfeatures".

I'm now sorry that I used the word "misfeature" because it drew your
attention away from the real objection.  But since you asked: the
misfeature here is that `select' works only on sockets.

>   - I don't think that answering your objection with an autoconf test for
>     WaitForMultipleObjects would make you any happier.  Would it?

No.  The real issue is that WIN32API is an alias for _WIN32 and its
ilk, something we spent some time weeding out several months ago; and
that the code inside the ifdef calls WaitForMultipleObjects, which is
a Windows-specific function not standardized anywhere else.

> How does the fact that only Windows provides WaitForMultipleObjects
> make it conceptually different from the fact that a bunch of systems
> provide poll, and a bunch don't?

The difference is that `select' and `poll' are both Posix functions,
while WaitForMultipleObjects is MS-Windows specific.

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Thu, 21 Apr 2005 14:15:28 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> CC: Eli Zaretskii <eliz@gnu.org>,  gdb-patches@sources.redhat.com
> 
> Daniel Jacobowitz wrote:
> 
> > Anyway, if Mark can come up with a select wrapper, then maybe we can
> > drop the question entirely.
> 
> I'm sure I can, if that's the only way to get this functionality into 
> GDB.

It's not the _only_ way, but it's the _preferred_ way, at least in my
opinion.  (If other maintaners, besides Daniel, disagree with me,
please speak up.)  If it becomse clear that doing what I asked is
going to be a major project, I assure you I will withdraw my
objections right there and then.

> But, the emacs select wrapper is hardly a full implementation of 
> select; for example, it only handles file descriptors waiting for 
> "read", not "write".

AFAIK, GDB also needs to watch handles only for readable events, not
for writable events.  You will see that does not add the GDB_WRITABLE
bit to the mask it passes to select/poll.  If we wish to guard against
possible changes in the future that will break the emulation, we could
add some code that will yell bloody murder if select is ever called to
watch handles for writability.

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Fri, Apr 22, 2005 at 11:22:56AM +0300, Eli Zaretskii wrote:
>> Date: Thu, 21 Apr 2005 14:15:28 -0700
>> From: Mark Mitchell <mark@codesourcery.com>
>> CC: Eli Zaretskii <eliz@gnu.org>,  gdb-patches@sources.redhat.com
>> 
>> Daniel Jacobowitz wrote:
>> 
>> > Anyway, if Mark can come up with a select wrapper, then maybe we can
>> > drop the question entirely.
>> 
>> I'm sure I can, if that's the only way to get this functionality into 
>> GDB.
>
>It's not the _only_ way, but it's the _preferred_ way, at least in my
>opinion.  (If other maintaners, besides Daniel, disagree with me,
>please speak up.)  If it becomse clear that doing what I asked is
>going to be a major project, I assure you I will withdraw my
>objections right there and then.

Well, again, I have a rather major technical concern about the use of
WaitForMultipleObjects in this scenario, so as the Windows maintainer,
I'd like to see that addressed.  You can't reliably just use
WaitForMultiple on, say, a serial port, a socket, or a pipe, so I don't
know how this would ever work.

cgf

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Fri, 22 Apr 2005 08:08:04 -0400
> From: Christopher Faylor <me@cgf.cx>
> 
> Well, again, I have a rather major technical concern about the use of
> WaitForMultipleObjects in this scenario, so as the Windows maintainer,
> I'd like to see that addressed.  You can't reliably just use
> WaitForMultiple on, say, a serial port, a socket, or a pipe, so I don't
> know how this would ever work.

I'm not sure I understand what you are saying, Chris.  Are you saying
that there's no known way of emulating the Posix `select' on Windows
in a way that would work on serial ports and pipes?  (I assume sockets
are a non-issue, since the Windows implementation of `select' supports
them.)

Or are you saying that WaitForMultipleObjects is not the way to write
such an emulation?  If so, what system calls are better candidates?

FWIW, the Emacs emulation of `select' does work on pipes, so it seems
that at least in that case there's code to borrow.

If we cannot make a `select' emulation that works for some of these
devices, we could simply document them as a limitation.  That doesn't
make the Windows build worse than it is now, does it?

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Fri, Apr 22, 2005 at 04:20:40PM +0300, Eli Zaretskii wrote:
>> Date: Fri, 22 Apr 2005 08:08:04 -0400
>> From: Christopher Faylor <me@cgf.cx>
>> 
>> Well, again, I have a rather major technical concern about the use of
>> WaitForMultipleObjects in this scenario, so as the Windows maintainer,
>> I'd like to see that addressed.  You can't reliably just use
>> WaitForMultiple on, say, a serial port, a socket, or a pipe, so I don't
>> know how this would ever work.
>
>I'm not sure I understand what you are saying, Chris.  Are you saying
>that there's no known way of emulating the Posix `select' on Windows
>in a way that would work on serial ports and pipes?  (I assume sockets
>are a non-issue, since the Windows implementation of `select' supports
>them.)

I'm saying that you can't just use WaitForMultipleObjects as a
replacement for select.  You have to go to more effort than that,
unfortunately.

AFAIK, the only file-like device that can be used in
WaitForMultipleObjects is the windows console.

>Or are you saying that WaitForMultipleObjects is not the way to write
>such an emulation?  If so, what system calls are better candidates?
>
>FWIW, the Emacs emulation of `select' does work on pipes, so it seems
>that at least in that case there's code to borrow.

Cygwin's emulation does too, but it doesn't use WaitForMultipleObjects.
AFAIK, the only way to do non-blocking reads on normal pipes is to poll
the pipe with the PeekNamedPipe call.  It's ugly and painful.

The Windows API is full of interesting and useful stuff mixed together
with inexplicably annoying and limited stuff.

>If we cannot make a `select' emulation that works for some of these
>devices, we could simply document them as a limitation.  That doesn't
>make the Windows build worse than it is now, does it?

Cygwin has a fully functional select (at least for the read side of
things) so it is possible to come close.

FWIW, you can do non-blocking I/O on serial handles by opening them in
"overlapped" mode but I don't know if there's any way to do that via
the UNIX-lite "fopen/open" MSVCRT interface.

cgf

Re: PATCH: Support Windows in event-loop.c

[Ian Lance Taylor]

Christopher Faylor <me@cgf.cx> writes:

> >Or are you saying that WaitForMultipleObjects is not the way to write
> >such an emulation?  If so, what system calls are better candidates?
> >
> >FWIW, the Emacs emulation of `select' does work on pipes, so it seems
> >that at least in that case there's code to borrow.
> 
> Cygwin's emulation does too, but it doesn't use WaitForMultipleObjects.
> AFAIK, the only way to do non-blocking reads on normal pipes is to poll
> the pipe with the PeekNamedPipe call.  It's ugly and painful.

For what it's worth, Tcl implements an event loop on Windows which
works for all types of objects.  The main trick is to create a little
thread which sits around reading the pipe, and raises an event when it
gets something.  Of course, you then have to carefully hand that data
over to the main thread when the main thread wants to read from the
pipe.

Ian

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Fri, Apr 22, 2005 at 11:14:18AM -0400, Ian Lance Taylor wrote:
>Christopher Faylor <me@cgf.cx> writes:
>
>> >Or are you saying that WaitForMultipleObjects is not the way to write
>> >such an emulation?  If so, what system calls are better candidates?
>> >
>> >FWIW, the Emacs emulation of `select' does work on pipes, so it seems
>> >that at least in that case there's code to borrow.
>> 
>> Cygwin's emulation does too, but it doesn't use WaitForMultipleObjects.
>> AFAIK, the only way to do non-blocking reads on normal pipes is to poll
>> the pipe with the PeekNamedPipe call.  It's ugly and painful.
>
>For what it's worth, Tcl implements an event loop on Windows which
>works for all types of objects.  The main trick is to create a little
>thread which sits around reading the pipe, and raises an event when it
>gets something.  Of course, you then have to carefully hand that data
>over to the main thread when the main thread wants to read from the
>pipe.

That's roughly how cygwin does it, too.

cgf

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Christopher Faylor wrote:

> Well, again, I have a rather major technical concern about the use of
> WaitForMultipleObjects in this scenario, so as the Windows maintainer,
> I'd like to see that addressed.  You can't reliably just use
> WaitForMultiple on, say, a serial port, a socket, or a pipe, so I don't
> know how this would ever work.

Very fair question.  And, of course, packaging up WaitForMultipleObjects 
into select isn't going to be useful if it's not even the right API. 
With the current set of patches, which only support remote debugging, 
the only thing that shows up in the console input handle, for which you 
can use WaitForMultipleObjects.

To really implement select, with reasonable performance, I think you 
have to do actually spawn multiple threads, to use the appropriate way 
of waiting for different things.  My plan here was that when that 
situation arose we would have those threads signal an Event, which 
WaitForMultipleObjects can process.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Fri, Apr 22, 2005 at 11:13:51AM +0300, Eli Zaretskii wrote:
> > How does the fact that only Windows provides WaitForMultipleObjects
> > make it conceptually different from the fact that a bunch of systems
> > provide poll, and a bunch don't?
> 
> The difference is that `select' and `poll' are both Posix functions,
> while WaitForMultipleObjects is MS-Windows specific.

I guess I don't see this as a problem, while you do.  In any case,
since Chris has raised technical objections, I'm going to sit back and
see what the next revision looks like.  Hopefully it will make us both
happier.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Mark Kettenis]

   Date: Sun, 24 Apr 2005 18:18:06 -0400
   From: Daniel Jacobowitz <drow@false.org>

   On Fri, Apr 22, 2005 at 11:13:51AM +0300, Eli Zaretskii wrote:
   > > How does the fact that only Windows provides WaitForMultipleObjects
   > > make it conceptually different from the fact that a bunch of systems
   > > provide poll, and a bunch don't?
   > 
   > The difference is that `select' and `poll' are both Posix functions,
   > while WaitForMultipleObjects is MS-Windows specific.

   I guess I don't see this as a problem, while you do.  In any case,
   since Chris has raised technical objections, I'm going to sit back and
   see what the next revision looks like.  Hopefully it will make us both
   happier.

The problem here is that traditional gdb programmers are familliar
with POSIX and therefore certainly with select, and probably with
poll.  They might very well not be familliar with Windows and the
magic of WaitForMultipleObjects.  If we accept the patch as-is, the
number of people that can hack on the event loop code decreases since
the number of people who are familliar with both Windows and POSIX is
smaller than the number of people who know about POSIX.  This is why I
think it would be preferable if an emulation for select or poll would
be used here.

Mark

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Daniel Jacobowitz wrote:

> I guess I don't see this as a problem, while you do.  In any case,
> since Chris has raised technical objections, I'm going to sit back and
> see what the next revision looks like.  Hopefully it will make us both
> happier.

I, too, am waiting on Chris' comments re. my justification for using 
WaitForMultipleObjects.  If it turns out that this is not the right 
primitive to use, then we'll have to revisit that side of things, but I 
suspect that the choice between directly modifying the file and 
providing a (almost-certainly incomplete) implementation of "select" 
will probably remain.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Mark Kettenis wrote:

> The problem here is that traditional gdb programmers are familliar
> with POSIX and therefore certainly with select, and probably with
> poll.  They might very well not be familliar with Windows and the
> magic of WaitForMultipleObjects.  If we accept the patch as-is, the
> number of people that can hack on the event loop code decreases since
> the number of people who are familliar with both Windows and POSIX is
> smaller than the number of people who know about POSIX.  This is why I
> think it would be preferable if an emulation for select or poll would
> be used here.

The flip side of this argument is that unless we can provide something 
that really does behave like select, people will *think* they understand 
what's going on in this code, but won't.  For example, if we end up with 
a select that can't handle waiting for "write" events on file 
descriptors, then at some point someone might add such a thing, and then 
Windows will break.

This is a somewhat different situation from the sockets situation, where 
the Windows API is very nearly identical to the UNIX API, modulo 
spelling, for the purposes of GDB.  Intuition about UNIX sockets applies 
to Windows sockets, but intuition about UNIX file descriptors and select 
doesn't apply all that well to Windows.

I have a feeling, though, that I'm shouting into the wind.  I understand 
that the GDB community is POSIX-centric and of course GNU is Not UNIX, 
rather than Not Windows.  I understand that, and it's much more 
important to me that the functionality be provided than that any 
particular form be used.

I think my current intent is to wait for Chris' comments re. 
WaitForMultipleObjects and then try to make something that works like a 
limited form of select.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Sun, Apr 24, 2005 at 04:57:36PM -0700, Mark Mitchell wrote:
>Daniel Jacobowitz wrote:
>>I guess I don't see this as a problem, while you do.  In any case,
>>since Chris has raised technical objections, I'm going to sit back and
>>see what the next revision looks like.  Hopefully it will make us both
>>happier.
>
>I, too, am waiting on Chris' comments re.  my justification for using
>WaitForMultipleObjects.  If it turns out that this is not the right
>primitive to use, then we'll have to revisit that side of things, but I
>suspect that the choice between directly modifying the file and
>providing a (almost-certainly incomplete) implementation of "select"
>will probably remain.

Sorry.  I didn't know you were waiting for any further feedback from me.

Your last message implied that the only handle which ever makes it into
a select call is a console handle.  Is that right?  I thought that it
was also used for serial I/O and GDB/MI.

cgf

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Mon, Apr 25, 2005 at 12:24:14AM -0400, Christopher Faylor wrote:
> On Sun, Apr 24, 2005 at 04:57:36PM -0700, Mark Mitchell wrote:
> >Daniel Jacobowitz wrote:
> >>I guess I don't see this as a problem, while you do.  In any case,
> >>since Chris has raised technical objections, I'm going to sit back and
> >>see what the next revision looks like.  Hopefully it will make us both
> >>happier.
> >
> >I, too, am waiting on Chris' comments re.  my justification for using
> >WaitForMultipleObjects.  If it turns out that this is not the right
> >primitive to use, then we'll have to revisit that side of things, but I
> >suspect that the choice between directly modifying the file and
> >providing a (almost-certainly incomplete) implementation of "select"
> >will probably remain.
> 
> Sorry.  I didn't know you were waiting for any further feedback from me.
> 
> Your last message implied that the only handle which ever makes it into
> a select call is a console handle.  Is that right?  I thought that it
> was also used for serial I/O and GDB/MI.

It is only the console handle - in the configuration that Mark's been
testing.

Currently, we always select for READ|EXCEPT.  MI uses it for stdin (on
Unix this would be either a console for testing, or a pipe); GDB and
TUI use it for the console; and ser-base uses it for serial IO.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Mon, Apr 25, 2005 at 09:16:12AM -0400, Daniel Jacobowitz wrote:
>On Mon, Apr 25, 2005 at 12:24:14AM -0400, Christopher Faylor wrote:
>> On Sun, Apr 24, 2005 at 04:57:36PM -0700, Mark Mitchell wrote:
>> >Daniel Jacobowitz wrote:
>> >>I guess I don't see this as a problem, while you do.  In any case,
>> >>since Chris has raised technical objections, I'm going to sit back and
>> >>see what the next revision looks like.  Hopefully it will make us both
>> >>happier.
>> >
>> >I, too, am waiting on Chris' comments re.  my justification for using
>> >WaitForMultipleObjects.  If it turns out that this is not the right
>> >primitive to use, then we'll have to revisit that side of things, but I
>> >suspect that the choice between directly modifying the file and
>> >providing a (almost-certainly incomplete) implementation of "select"
>> >will probably remain.
>> 
>> Sorry.  I didn't know you were waiting for any further feedback from me.
>> 
>> Your last message implied that the only handle which ever makes it into
>> a select call is a console handle.  Is that right?  I thought that it
>> was also used for serial I/O and GDB/MI.
>
>It is only the console handle - in the configuration that Mark's been
>testing.

Ok...  So, is it acceptable to include a console-only implementation in
event-loop.c?  I would think that it wasn't.

That seems to suggest that some kind of generic select or poll
implementation needs to be developed, probably using threads.

cgf

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Christopher Faylor wrote:

> Ok...  So, is it acceptable to include a console-only implementation in
> event-loop.c?  I would think that it wasn't.
> 
> That seems to suggest that some kind of generic select or poll
> implementation needs to be developed, probably using threads.

The second part of my claim seems to have gotten lost.  In particular, 
*at present* the only handle is the console, so WaitForMultipleObjects 
works fine.  In future, there may be other handles; my plan was that for 
any handle for which WaitForMultipleObjects did not work directly, we 
would create an Event, and a thread that wait for the appropriate thing 
to happen and signal the Event.  Since WaitForMultipleObjects works with 
Events, that would still be the right primitive to actually detect what 
happened.

All that would need to change relative to the current code would be to 
create/destroy the threads as necessary.  So, the current implementation 
is only console-only in that some details haven't been added, not in 
that it's hardwired in some permanent way to consoles.

Does that seem like a workable plan to you?

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Mon, Apr 25, 2005 at 07:59:34AM -0700, Mark Mitchell wrote:
> Christopher Faylor wrote:
> 
> >Ok...  So, is it acceptable to include a console-only implementation in
> >event-loop.c?  I would think that it wasn't.
> >
> >That seems to suggest that some kind of generic select or poll
> >implementation needs to be developed, probably using threads.
> 
> The second part of my claim seems to have gotten lost.  In particular, 
> *at present* the only handle is the console, so WaitForMultipleObjects 
> works fine.

Can you provide some evidence for this?  I don't think it's true, as I
said in my earlier message.  The current GDB seems to use it for both
Windows serial ports and for the MI console, which is likely to be a
pipe rather than a true console.

> In future, there may be other handles; my plan was that for 
> any handle for which WaitForMultipleObjects did not work directly, we 
> would create an Event, and a thread that wait for the appropriate thing 
> to happen and signal the Event.  Since WaitForMultipleObjects works with 
> Events, that would still be the right primitive to actually detect what 
> happened.
> 
> All that would need to change relative to the current code would be to 
> create/destroy the threads as necessary.  So, the current implementation 
> is only console-only in that some details haven't been added, not in 
> that it's hardwired in some permanent way to consoles.
> 
> Does that seem like a workable plan to you?

I don't think we should add something this limited.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Daniel Jacobowitz wrote:

>>Does that seem like a workable plan to you?
>  
> I don't think we should add something this limited.

OK.  I'll go code up something select-like for Windows.

It will not be fully general because each new kind of event we want to 
wait for will in general require a new API call, and I can't enumerate 
what the eventual complete set might be.  (In UNIX, everything is a file 
descriptor from this point of view; not true in Windows.)  However, it 
will at least have the right interface.

I'm not personally aware of how to get anything other than a console 
handle into this loop.  Can you give me a test procedure that should get 
me something else, given the currently limited functionality on 
(non-Cygwin) Windows?  Otherwise, I'd still prefer to add the generality 
to select once I can test what I'm dealing with.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Mon, Apr 25, 2005 at 08:17:37AM -0700, Mark Mitchell wrote:
> Daniel Jacobowitz wrote:
> 
> >>Does that seem like a workable plan to you?
> > 
> >I don't think we should add something this limited.
> 
> OK.  I'll go code up something select-like for Windows.
> 
> It will not be fully general because each new kind of event we want to 
> wait for will in general require a new API call, and I can't enumerate 
> what the eventual complete set might be.  (In UNIX, everything is a file 
> descriptor from this point of view; not true in Windows.)  However, it 
> will at least have the right interface.

Can't you borrow Cygwin's, or the one from Emacs that Eli referenced? 
No need to reinvent the wheel.

> I'm not personally aware of how to get anything other than a console 
> handle into this loop.  Can you give me a test procedure that should get 
> me something else, given the currently limited functionality on 
> (non-Cygwin) Windows?  Otherwise, I'd still prefer to add the generality 
> to select once I can test what I'm dealing with.

Try running gdb -i=mi piped to and from cat?

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Daniel Jacobowitz wrote:

> Can't you borrow Cygwin's, or the one from Emacs that Eli referenced? 
> No need to reinvent the wheel.

Yes, I'll try that.  But, I don't want people to get false expectations; 
the one from Emacs is very incomplete.  The one from Cygwin is very 
complete, but also appears complex and dependent on lots of other bits 
of Cygwin.

Now, assuming I can do this, do y'all want me to try to put this in 
libiberty, or in GDB itself?

>>I'm not personally aware of how to get anything other than a console 
>>handle into this loop.  Can you give me a test procedure that should get 
>>me something else, given the currently limited functionality on 
>>(non-Cygwin) Windows?  Otherwise, I'd still prefer to add the generality 
>>to select once I can test what I'm dealing with.
> 
> 
> Try running gdb -i=mi piped to and from cat?

OK.  Thanks.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Mon, Apr 25, 2005 at 08:26:21AM -0700, Mark Mitchell wrote:
> Daniel Jacobowitz wrote:
> 
> >Can't you borrow Cygwin's, or the one from Emacs that Eli referenced? 
> >No need to reinvent the wheel.
> 
> Yes, I'll try that.  But, I don't want people to get false expectations; 
> the one from Emacs is very incomplete.  The one from Cygwin is very 
> complete, but also appears complex and dependent on lots of other bits 
> of Cygwin.
> 
> Now, assuming I can do this, do y'all want me to try to put this in 
> libiberty, or in GDB itself?

No idea.  I think in GDB is OK.  In that case it should probably be
encapsulated as "gdb_select".

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Ian Lance Taylor]

Mark Mitchell <mark@codesourcery.com> writes:

> Now, assuming I can do this, do y'all want me to try to put this in
> libiberty, or in GDB itself?

I suspect that a select which can works on Windows named pipes can
only be implemented in conjunction with an I/O library for reading
from the pipes.  This is because to make select work properly, the
thread which is responsible for the pipe has to actually read some
data from the pipe.  At least the last time I looked, there is no way
to wait for a pipe to have some data available.  There is a
PeekNamedPipe (I think) call which can check whether there is data
available, but the only way to use that is to busy wait.  So that
means that the thread will have read some data which the main program
wants.  And the only reasonable way I see to handle that is to provide
an interface to read from the descriptor which will check whether any
data was read by the read.

In other words, you need not just a select emulation, but also a read
emulation, and the two have to work together.  And really to make that
work reasonably, you will need to emulate open, close and write also.
And that we get into socket and pipe.

So that is crazy.  We don't want to emulate select.  I think it would
make more sense to add an argument to add_file_handler which indicates
the type of the file descriptor.  Then split up event-loop.c into
generic code, Unix specific code, and Windows specific code.

Ian

Re: PATCH: Support Windows in event-loop.c

[M.M. Kettenis]

Christopher Faylor <me@cgf.cx> wrote: 

> On Mon, Apr 25, 2005 at 09:16:12AM -0400, Daniel Jacobowitz wrote:
> >On Mon, Apr 25, 2005 at 12:24:14AM -0400, Christopher Faylor wrote:
> >> On Sun, Apr 24, 2005 at 04:57:36PM -0700, Mark Mitchell wrote:
> >> >Daniel Jacobowitz wrote:

> Ok...  So, is it acceptable to include a console-only implementation in
> event-loop.c?  I would think that it wasn't.
> 
> That seems to suggest that some kind of generic select or poll
> implementation needs to be developed, probably using threads.

Over my dead body (the threads part that is).

Mark

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Mon, Apr 25, 2005 at 03:51:41PM +0000, M.M. Kettenis wrote:
> Christopher Faylor <me@cgf.cx> wrote: 
> 
> >On Mon, Apr 25, 2005 at 09:16:12AM -0400, Daniel Jacobowitz wrote:
> >>On Mon, Apr 25, 2005 at 12:24:14AM -0400, Christopher Faylor wrote:
> >>> On Sun, Apr 24, 2005 at 04:57:36PM -0700, Mark Mitchell wrote:
> >>> >Daniel Jacobowitz wrote:
> 
> >Ok...  So, is it acceptable to include a console-only implementation in
> >event-loop.c?  I would think that it wasn't.
> >
> >That seems to suggest that some kind of generic select or poll
> >implementation needs to be developed, probably using threads.
> 
> Over my dead body (the threads part that is).

Unfortunately there appears to be exactly zero other ways to do this in
Windows.

-- 
Daniel Jacobowitz
CodeSourcery, LLC

Re: PATCH: Support Windows in event-loop.c

[Ian Lance Taylor]

> > Ok...  So, is it acceptable to include a console-only implementation in
> > event-loop.c?  I would think that it wasn't.
> > That seems to suggest that some kind of generic select or poll
> > implementation needs to be developed, probably using threads.
> 
> Over my dead body (the threads part that is).

It has to be done with threads on Windows.  But the threads don't
escape from the select emulation.  They are just tiny little threads
which sit around waiting for something to happen on a file descriptor,
and which post an event when it does.

Ian

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Mon, Apr 25, 2005 at 12:08:16PM -0400, Ian Lance Taylor wrote:
>> > Ok...  So, is it acceptable to include a console-only implementation in
>> > event-loop.c?  I would think that it wasn't.
>> > That seems to suggest that some kind of generic select or poll
>> > implementation needs to be developed, probably using threads.
>> 
>> Over my dead body (the threads part that is).
>
>It has to be done with threads on Windows.  But the threads don't
>escape from the select emulation.  They are just tiny little threads
>which sit around waiting for something to happen on a file descriptor,
>and which post an event when it does.

If we were comfortable with the concept of polling, I don't see any
reason why this couldn't be done in a polling loop rather than using
threads.  If polling isn't an option then the only way to do this
is with threads.

The cygwin select code might provide a clue as to how we do some of this
but I think there might be ownership issues with using it in gdb.  I'd
have no problem including it but getting someone at Red Hat to focus on
the issue long enough to provide a statement of assent would probably be
difficult.

cgf

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 10:50:23 -0400
> From: Christopher Faylor <me@cgf.cx>
> 
> >It is only the console handle - in the configuration that Mark's been
> >testing.
> 
> Ok...  So, is it acceptable to include a console-only implementation in
> event-loop.c?  I would think that it wasn't.

Unless a more general emulation can be coded with a reasonable amount
of work, I'd be prepared to go along with a console-only version.  We
could add something there that will abort if a non-console handle is
ever passed, if we want.

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 11:35:49 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Christopher Faylor <me@cgf.cx>, gdb-patches@sources.redhat.com
> 
> > Now, assuming I can do this, do y'all want me to try to put this in 
> > libiberty, or in GDB itself?
> 
> No idea.  I think in GDB is OK.  In that case it should probably be
> encapsulated as "gdb_select".

Yes, that would be my recommendation as well.

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

M.M. Kettenis wrote:

>> That seems to suggest that some kind of generic select or poll
>> implementation needs to be developed, probably using threads.
> 
> Over my dead body (the threads part that is).

There is no way to implement the full generality of select on Windows 
without threads.

Based on all the feedback, what I plan to do is simply take the code 
I've written, package it up in gdb_select, implementing support for 
consoles, and declare victory.  That avoids using non-POSIX APIs in 
common code, and provides useful functionality on Windows.  It may not 
support everything (like the MI), that's no worse than the current 
state.  Then, we can argue about how/if to use threads to support more 
generality.

If there are any objections to the above plan, I'd appreciate hearing 
them soon, as I'll be working on this code later today.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

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

Daniel Jacobowitz wrote:

> No idea.  I think in GDB is OK.  In that case it should probably be
> encapsulated as "gdb_select".

Here is the revised patch.  As per previous discussion, this version 
still has the limitation that it only works with consoles -- but it 
would be possible to use threads within gdb_select to extend it to work 
with a wider variety of handles.  In any case, this patch provides 
useful functionality on Windows, and does not impose the Windows API on 
generic code.

OK to commit?

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

[-- Attachment #2: gdb.event.patch --]
[-- Type: text/plain, Size: 7124 bytes --]

2005-04-25  Mark Mitchell  <mark@codesourcery.com>

	* event-loop.c (gdb_assert.h): Include.
	(<windows.h>): Include under Windows.
	(<io.h>): Likeiwse.
	(gdb_select): New function.
	(gdb_wait_for_event): Use it.
	* Makefile.in (event-loop.o): Depend on $(gdb_assert_h).
	
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.707
diff -c -5 -p -r1.707 Makefile.in
*** Makefile.in	18 Mar 2005 21:03:38 -0000	1.707
--- Makefile.in	25 Apr 2005 20:09:04 -0000
*************** environ.o: environ.c $(defs_h) $(environ
*** 1907,1917 ****
  eval.o: eval.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
  	$(value_h) $(expression_h) $(target_h) $(frame_h) $(language_h) \
  	$(f_lang_h) $(cp_abi_h) $(infcall_h) $(objc_lang_h) $(block_h) \
  	$(parser_defs_h) $(cp_support_h)
  event-loop.o: event-loop.c $(defs_h) $(event_loop_h) $(event_top_h) \
! 	$(gdb_string_h) $(exceptions_h)
  event-top.o: event-top.c $(defs_h) $(top_h) $(inferior_h) $(target_h) \
  	$(terminal_h) $(event_loop_h) $(event_top_h) $(interps_h) \
  	$(exceptions_h) $(gdbcmd_h) $(readline_h) $(readline_history_h)
  exceptions.o: exceptions.c $(defs_h) $(exceptions_h) $(breakpoint_h) \
  	$(target_h) $(inferior_h) $(annotate_h) $(ui_out_h) $(gdb_assert_h) \
--- 1910,1920 ----
  eval.o: eval.c $(defs_h) $(gdb_string_h) $(symtab_h) $(gdbtypes_h) \
  	$(value_h) $(expression_h) $(target_h) $(frame_h) $(language_h) \
  	$(f_lang_h) $(cp_abi_h) $(infcall_h) $(objc_lang_h) $(block_h) \
  	$(parser_defs_h) $(cp_support_h)
  event-loop.o: event-loop.c $(defs_h) $(event_loop_h) $(event_top_h) \
! 	$(gdb_string_h) $(exceptions_h) $(gdb_assert_h)
  event-top.o: event-top.c $(defs_h) $(top_h) $(inferior_h) $(target_h) \
  	$(terminal_h) $(event_loop_h) $(event_top_h) $(interps_h) \
  	$(exceptions_h) $(gdbcmd_h) $(readline_h) $(readline_history_h)
  exceptions.o: exceptions.c $(defs_h) $(exceptions_h) $(breakpoint_h) \
  	$(target_h) $(inferior_h) $(annotate_h) $(ui_out_h) $(gdb_assert_h) \
Index: event-loop.c
===================================================================
RCS file: /cvs/src/src/gdb/event-loop.c,v
retrieving revision 1.24
diff -c -5 -p -r1.24 event-loop.c
*** event-loop.c	12 Feb 2005 00:39:18 -0000	1.24
--- event-loop.c	25 Apr 2005 20:20:34 -0000
***************
*** 34,43 ****
--- 34,44 ----
  #include <sys/types.h>
  #include "gdb_string.h"
  #include <errno.h>
  #include <sys/time.h>
  #include "exceptions.h"
+ #include "gdb_assert.h"
  
  typedef struct gdb_event gdb_event;
  typedef void (event_handler_func) (int);
  
  /* Event for the GDB event system.  Events are queued by calling
*************** event_queue;
*** 131,140 ****
--- 132,146 ----
  #define USE_POLL 0
  #endif /* HAVE_POLL */
  
  static unsigned char use_poll = USE_POLL;
  
+ #ifdef USE_WIN32API
+ #include <windows.h>
+ #include <io.h>
+ #endif
+ 
  static struct
    {
      /* Ptr to head of file handler list. */
      file_handler *first_file_handler;
  
*************** handle_file_event (int event_file_desc)
*** 723,732 ****
--- 729,806 ----
  	  break;
  	}
      }
  }
  
+ /* Wrapper for select.  This function is not yet exported from this
+    file because it is not sufficiently general.  For example,
+    ser-base.c uses select to check for socket activity, and this
+    function does not support sockets under Windows, so we do not want
+    to use gdb_select in ser-base.c.  */
+ 
+ static int 
+ gdb_select (int n, fd_set *readfds, fd_set *writefds, fd_set *exceptfds,
+ 	    struct timeval *timeout)
+ {
+ #ifdef USE_WIN32API
+   HANDLE handles[MAXIMUM_WAIT_OBJECTS];
+   HANDLE h;
+   DWORD event;
+   DWORD num_handles;
+   int fd;
+ 
+   num_handles = 0;
+   for (fd = 0; fd < n; ++fd)
+     {
+       /* EXCEPTFDS is silently ignored.  GDB always sets GDB_EXCEPTION
+ 	 when calling add_file_handler, but there is no natural analog
+ 	 under Windows.  */
+       /* There is no support yet for WRITEFDS.  At present, this isn't
+ 	 used by GDB -- but we do not want to silently ignore WRITEFDS
+ 	 if something starts using it.  */
+       gdb_assert (!FD_ISSET (fd, writefds));
+       if (FD_ISSET (fd, readfds))
+ 	handles[num_handles++] = (HANDLE) _get_osfhandle (fd);
+     }
+   event = WaitForMultipleObjects (num_handles,
+ 				  handles,
+ 				  FALSE,
+ 				  timeout 
+ 				  ? (timeout->tv_sec * 1000 + timeout->tv_usec)
+ 				  : INFINITE);
+   /* EVENT can only be a value in the WAIT_ABANDONED_0 range if the
+      HANDLES included an abandoned mutex.  Since GDB doesn't use
+      mutexes, that should never occur.  */
+   gdb_assert (!(WAIT_ABANDONED_0 <= event
+ 		&& event < WAIT_ABANDONED_0 + num_handles));
+   if (event == WAIT_FAILED)
+     return -1;
+   if (event == WAIT_TIMEOUT)
+     return 0;
+   /* Run through the READFDS, clearing bits corresponding to descriptors
+      for which input is unavailable.  */
+   h = handles[event - WAIT_OBJECT_0];
+   for (fd = 0; fd < n; ++fd)
+     {
+       HANDLE fd_h = (HANDLE) _get_osfhandle (fd);
+       /* This handle might be ready, even though it wasn't the handle
+ 	 returned by WaitForMultipleObjects.  */
+       if (FD_ISSET (fd, readfds) && fd_h != h
+ 	  && WaitForSingleObject (fd_h, 0) == WAIT_OBJECT_0)
+ 	FD_CLR (fd, readfds);
+     }
+   /* We never report any descriptors available for writing or with
+      exceptional conditions.  */ 
+   FD_ZERO (writefds);
+   FD_ZERO (exceptfds);
+ 
+   return 1;
+ #else
+   return select (n, readfds, writefds, exceptfds, timeout);
+ #endif
+ }
+ 
  /* Called by gdb_do_one_event to wait for new events on the 
     monitored file descriptors. Queue file events as they are 
     detected by the poll. 
     If there are no events, this function will block in the 
     call to poll.
*************** gdb_wait_for_event (void)
*** 767,782 ****
    else
      {
        gdb_notifier.ready_masks[0] = gdb_notifier.check_masks[0];
        gdb_notifier.ready_masks[1] = gdb_notifier.check_masks[1];
        gdb_notifier.ready_masks[2] = gdb_notifier.check_masks[2];
!       num_found = select (gdb_notifier.num_fds,
! 			  &gdb_notifier.ready_masks[0],
! 			  &gdb_notifier.ready_masks[1],
! 			  &gdb_notifier.ready_masks[2],
! 			  gdb_notifier.timeout_valid
! 			  ? &gdb_notifier.select_timeout : NULL);
  
        /* Clear the masks after an error from select. */
        if (num_found == -1)
  	{
  	  FD_ZERO (&gdb_notifier.ready_masks[0]);
--- 841,856 ----
    else
      {
        gdb_notifier.ready_masks[0] = gdb_notifier.check_masks[0];
        gdb_notifier.ready_masks[1] = gdb_notifier.check_masks[1];
        gdb_notifier.ready_masks[2] = gdb_notifier.check_masks[2];
!       num_found = gdb_select (gdb_notifier.num_fds,
! 			      &gdb_notifier.ready_masks[0],
! 			      &gdb_notifier.ready_masks[1],
! 			      &gdb_notifier.ready_masks[2],
! 			      gdb_notifier.timeout_valid
! 			      ? &gdb_notifier.select_timeout : NULL);
  
        /* Clear the masks after an error from select. */
        if (num_found == -1)
  	{
  	  FD_ZERO (&gdb_notifier.ready_masks[0]);

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 10:07:42 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> CC: Christopher Faylor <me@cgf.cx>,  gdb-patches@sources.redhat.com
> 
> It may not support everything (like the MI), that's no worse than
> the current state.

Indeed.

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 13:22:16 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> CC: Christopher Faylor <me@cgf.cx>,  gdb-patches@sources.redhat.com
> 
> Here is the revised patch.  As per previous discussion, this version 
> still has the limitation that it only works with consoles -- but it 
> would be possible to use threads within gdb_select to extend it to work 
> with a wider variety of handles.  In any case, this patch provides 
> useful functionality on Windows, and does not impose the Windows API on 
> generic code.

This patch is fine with me.

Thanks.

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Eli Zaretskii wrote:
>>Date: Mon, 25 Apr 2005 13:22:16 -0700
>>From: Mark Mitchell <mark@codesourcery.com>
>>CC: Christopher Faylor <me@cgf.cx>,  gdb-patches@sources.redhat.com
>>
>>Here is the revised patch.  As per previous discussion, this version 
>>still has the limitation that it only works with consoles -- but it 
>>would be possible to use threads within gdb_select to extend it to work 
>>with a wider variety of handles.  In any case, this patch provides 
>>useful functionality on Windows, and does not impose the Windows API on 
>>generic code.
> 
> 
> This patch is fine with me.

Thanks, committed.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Mark Kettenis]

   Date: Mon, 25 Apr 2005 14:46:00 -0700
   From: Mark Mitchell <mark@codesourcery.com>

   Eli Zaretskii wrote:
   >>Date: Mon, 25 Apr 2005 13:22:16 -0700
   >>From: Mark Mitchell <mark@codesourcery.com>
   >>CC: Christopher Faylor <me@cgf.cx>,  gdb-patches@sources.redhat.com
   >>
   >>Here is the revised patch.  As per previous discussion, this version 
   >>still has the limitation that it only works with consoles -- but it 
   >>would be possible to use threads within gdb_select to extend it to work 
   >>with a wider variety of handles.  In any case, this patch provides 
   >>useful functionality on Windows, and does not impose the Windows API on 
   >>generic code.
   > 
   > 
   > This patch is fine with me.

   Thanks, committed.

Thanks!  That was the last patch, isn't it?  If so, can you add some
blurp to NEWS about it?

Mark

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Mark Kettenis wrote:

>    Thanks, committed.
> 
> Thanks!  That was the last patch, isn't it?  If so, can you add some
> blurp to NEWS about it?

Not *quite* the last patch.  There are still readline changes required. 
  By and large, I should think these will be rather less controversial; 
most of them are just using autoconf macros to avoid using stuff that 
Windows doesn't have.  There is also one patch that basically says 
"don't use any of that termcap stuff" -- but that's really just a 
Makefile patch not to link in one file under Windows.

I understand that the readline used by GDB contains some GDB-local 
patches.  So, I'm planning to submit my patches to this list, too.  I 
know that normally we'd want to go to the upstream source, but Daniel 
says that's probably not practical in this case.

So, is it OK to submit readline patches here?

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Mark Kettenis]

   Date: Mon, 25 Apr 2005 15:08:46 -0700
   From: Mark Mitchell <mark@codesourcery.com>

   Mark Kettenis wrote:

   >    Thanks, committed.
   > 
   > Thanks!  That was the last patch, isn't it?  If so, can you add some
   > blurp to NEWS about it?

   Not *quite* the last patch.  There are still readline changes required. 
     By and large, I should think these will be rather less controversial; 
   most of them are just using autoconf macros to avoid using stuff that 
   Windows doesn't have.  There is also one patch that basically says 
   "don't use any of that termcap stuff" -- but that's really just a 
   Makefile patch not to link in one file under Windows.

   I understand that the readline used by GDB contains some GDB-local 
   patches.  So, I'm planning to submit my patches to this list, too.  I 
   know that normally we'd want to go to the upstream source, but Daniel 
   says that's probably not practical in this case.

   So, is it OK to submit readline patches here?

I'd say, please make an effort to get them into the official readline
first.  While we have some local readline patches, we should really
try to keep the number of them to the minimum.  Every local patch
makes merging the next version of readline more difficult :-(.  Once
the stuff is accepted upstream, no one should stop you from importing
those changes into GDB's readline.

If that really is not practical, try to keep the impact on existing
files as minimal as possible.  Try to keep things localized,
preferably in a seperate source files.  That will make life much
easier for us in the future.

Mark

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

> I'd say, please make an effort to get them into the official readline
> first. 

OK, I'll give it a shot.

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Mon, Apr 25, 2005 at 02:46:00PM -0700, Mark Mitchell wrote:
>Eli Zaretskii wrote:
>>>Date: Mon, 25 Apr 2005 13:22:16 -0700
>>>From: Mark Mitchell <mark@codesourcery.com>
>>>
>>>Here is the revised patch.  As per previous discussion, this version 
>>>still has the limitation that it only works with consoles -- but it 
>>>would be possible to use threads within gdb_select to extend it to work 
>>>with a wider variety of handles.  In any case, this patch provides 
>>>useful functionality on Windows, and does not impose the Windows API on 
>>>generic code.
>>
>>
>>This patch is fine with me.
>
>Thanks, committed.

Wow.  That was fast.

The patch was ok with me, too, but it seems like you should be checking
the bounds of the handles array just in the off chance that readfds is >
63.  I know that it is probably vanishingly unlikely that this could ever
occur but it still seems like good practice nonetheless.

cgf

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Christopher Faylor wrote:
> On Mon, Apr 25, 2005 at 02:46:00PM -0700, Mark Mitchell wrote:
> 
>>Eli Zaretskii wrote:
>>
>>>>Date: Mon, 25 Apr 2005 13:22:16 -0700
>>>>From: Mark Mitchell <mark@codesourcery.com>
>>>>
>>>>Here is the revised patch.  As per previous discussion, this version 
>>>>still has the limitation that it only works with consoles -- but it 
>>>>would be possible to use threads within gdb_select to extend it to work 
>>>>with a wider variety of handles.  In any case, this patch provides 
>>>>useful functionality on Windows, and does not impose the Windows API on 
>>>>generic code.
>>>
>>>
>>>This patch is fine with me.
>>
>>Thanks, committed.
> 
> 
> Wow.  That was fast.

Sorry, did I misinterpret Eli's message?  I took it as a commit 
approval, but perhaps that was overly optimistic?

> The patch was ok with me, too, but it seems like you should be checking
> the bounds of the handles array just in the off chance that readfds is >
> 63.  I know that it is probably vanishingly unlikely that this could ever
> occur but it still seems like good practice nonetheless.

OK.  May I consider a patch to add the appropriate gdb_assert obvious?

Thanks,

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Christopher Faylor]

On Mon, Apr 25, 2005 at 04:19:36PM -0700, Mark Mitchell wrote:
>>The patch was ok with me, too, but it seems like you should be checking
>>the bounds of the handles array just in the off chance that readfds is >
>>63.  I know that it is probably vanishingly unlikely that this could ever
>>occur but it still seems like good practice nonetheless.
>
>OK.  May I consider a patch to add the appropriate gdb_assert obvious?

Sure.

cgf

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Christopher Faylor wrote:
> On Mon, Apr 25, 2005 at 04:19:36PM -0700, Mark Mitchell wrote:
> 
>>>The patch was ok with me, too, but it seems like you should be checking
>>>the bounds of the handles array just in the off chance that readfds is >
>>>63.  I know that it is probably vanishingly unlikely that this could ever
>>>occur but it still seems like good practice nonetheless.
>>
>>OK.  May I consider a patch to add the appropriate gdb_assert obvious?

Committed, thanks!

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 11:04:22 -0400
> From: Daniel Jacobowitz <drow@false.org>
> Cc: Christopher Faylor <me@cgf.cx>, gdb-patches@sources.redhat.com
> 
> > In future, there may be other handles; my plan was that for 
> > any handle for which WaitForMultipleObjects did not work directly, we 
> > would create an Event, and a thread that wait for the appropriate thing 
> > to happen and signal the Event.  Since WaitForMultipleObjects works with 
> > Events, that would still be the right primitive to actually detect what 
> > happened.
> > 
> > All that would need to change relative to the current code would be to 
> > create/destroy the threads as necessary.  So, the current implementation 
> > is only console-only in that some details haven't been added, not in 
> > that it's hardwired in some permanent way to consoles.
> > 
> > Does that seem like a workable plan to you?
> 
> I don't think we should add something this limited.

If console handles is the only type that can currently end up in
`select', I don't see why shouldn't we add such an emulation, however
limited.  It sounds like, according to everybody who participated in
this thread, any more able emulation is going to be much more complex,
and some of them even involve Mark K.'s dead body.  It doesn't seem
right to me to reject the patch because of theoretical deficiencies.

The code that got committed was virtually the same one suggested
originally, only in slightly different wrapping.

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Tue, 26 Apr 2005 00:28:08 +0200 (CEST)
> From: Mark Kettenis <mark.kettenis@xs4all.nl>
> CC: eliz@gnu.org, drow@false.org, me@cgf.cx, gdb-patches@sources.redhat.com
> 
>    So, is it OK to submit readline patches here?
> 
> I'd say, please make an effort to get them into the official readline
> first.

That's true in general, but perhaps if we saw the readline patches
first, we could give a more sound advice, including which parts of the
patch are unlikely to be accepted by upstream Readline maintainers,
based on prior experience.  (IIRC, the DJGPP port of GDB is one reason
for local patches in Readline.)

Re: PATCH: Support Windows in event-loop.c

[Eli Zaretskii]

> Date: Mon, 25 Apr 2005 16:19:36 -0700
> From: Mark Mitchell <mark@codesourcery.com>
> CC: gdb-patches@sources.redhat.com,  drow@false.org, 
>  Eli Zaretskii <eliz@gnu.org>
> 
> > Wow.  That was fast.
> 
> Sorry, did I misinterpret Eli's message?  I took it as a commit 
> approval, but perhaps that was overly optimistic?

You didn't misinterpret my message.  However, since event-loop.c
doesn't have a single responsible maintainer, it is generally
advisable to wait for a few hours so that other maintainers could have
an opportunity to comment on the patch.

Re: PATCH: Support Windows in event-loop.c

[Mark Mitchell]

Eli Zaretskii wrote:
>>Date: Mon, 25 Apr 2005 16:19:36 -0700
>>From: Mark Mitchell <mark@codesourcery.com>
>>CC: gdb-patches@sources.redhat.com,  drow@false.org, 
>> Eli Zaretskii <eliz@gnu.org>
>>
>>>Wow.  That was fast.
>>
>>Sorry, did I misinterpret Eli's message?  I took it as a commit 
>>approval, but perhaps that was overly optimistic?
> 
> 
> You didn't misinterpret my message.  However, since event-loop.c
> doesn't have a single responsible maintainer, it is generally
> advisable to wait for a few hours so that other maintainers could have
> an opportunity to comment on the patch.

OK.  I'm still learning about GDB's development culture; thanks for 
being patient.

-- 
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304

Re: PATCH: Support Windows in event-loop.c

[Daniel Jacobowitz]

On Tue, Apr 26, 2005 at 06:47:47AM +0300, Eli Zaretskii wrote:
> If console handles is the only type that can currently end up in
> `select', I don't see why shouldn't we add such an emulation, however
> limited.  It sounds like, according to everybody who participated in
> this thread, any more able emulation is going to be much more complex,
> and some of them even involve Mark K.'s dead body.  It doesn't seem
> right to me to reject the patch because of theoretical deficiencies.
> 
> The code that got committed was virtually the same one suggested
> originally, only in slightly different wrapping.

FYI, your assumption in the first paragraph is not valid.  Both
ser-base and MI can add non-console handles to the event loop.
I believe both bits are reachable on Windows; Mark just hasn't
encountered them yet.

We'll deal with it when someone does, I suppose.

-- 
Daniel Jacobowitz
CodeSourcery, LLC