Re: RFA: Various Windows (mingw32) additions, mostly relating to select or serial ports

[Eli Zaretskii <eliz at gnu dot org>]

> Date: Fri, 3 Feb 2006 17:05:29 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> The primary ugly bit of this patch is the select wrapper.  Windows has
> interfaces for all these things which map to Unix file descriptors, but
> while the Unix interfaces are actually compatible, the Windows interfaces
> are just designed along similar principles.  So you can handle a serial port
> in roughly the same way you handle a console window.... but only roughly.
> In fact you need to know what sort of device is behind each "file
> descriptor", in order to handle it appropriately.

Could you elaborate a bit?  I cannot easily see where that device
knowledge is present in the patch; it all looks to me like several
instances of the same code with almost identical flow.

In particular, I thought WaitForMultipleObjects could handle any kind
of handle, be it a pipe, a socket, a console, a process, or a file.

Even if the code is slightly different in that it calls different OS
API functions, cannot it all be expressed as the same code that uses a
function table indexed by the interface type?

> The one I'm least proud of is pipes - there does not appear to be a way to
> sleep and have the OS wake you when data is available on a pipe.

??? Again, doesn't WaitForMultipleObjects fit the bill?

> I've tested it.  I'm not sure what else to say about it.  Is it OK?  Are
> there things blatantly wrong with it that I've missed?  Are there particular
> bits that you think are just too ugly, and if so, do you have any
> suggestions on how to make them less ugly?

I'd like an explanation about this paradigm:

> +      wait_events[0] = state->stop_select;
> +      wait_events[1] = h;
> +
> +      event_index = WaitForMultipleObjects (2, wait_events, FALSE, INFINITE);
> +
> +      if (event_index == WAIT_OBJECT_0
> +	  || WaitForSingleObject (state->stop_select, 0) == WAIT_OBJECT_0)
> +	{
> +	  CloseHandle (state->stop_select);
> +	  return 0;
> +	}

(You have similar code in gdb_select and elsewhere.)  Why do you need
to wait for an object again with WaitForSingleObject, after you've
just waited for it in WaitForMultipleObjects?

Otherwise, I'm okay with these patches, except that...

>  - Windows serial support.  This definitely deserves a NEWS entry,
>    included.

...I don't think you included this entry.