Re: [RFA] mingw port: Allow use of cvs GDB on Windows 95

[Joel Brobecker <brobecker@adacore.com>]

> 2011-02-05  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	Allow use of mingw native on Windows 95 OS.
> 	* src/gdb/ser-mingw.c (CancelIo): New macro for dynamically loaded
> 	DLL entry.
> 	(ser_windows_close): Only call CancelIo if function exists.
> 	(_initialize_ser_windows): Use LoadLirary/GetProcAddress
> 	to check for existence of CancelIo function in kernel32 DLL.

I was hoping that someone with more Windows knowledge would be able
to review this patch, but oh well...

This patch looks OK to me (ie, no risk that I can see for the other
platforms).  Just a couple of remarks:

> +/* CancelIo is not available for Windows 95 OS,
> +   so we need to use LoadLibrary/GetProcAddress to avoid
> +   a startup failure.  */

I find that the length of your lines are consistently really short
and inconsistent.  I think it makes it harder to read.  The above
can be reformatted to:

/* CancelIo is not available for Windows 95 OS, so we need to use
   LoadLibrary/GetProcAddress to avoid a startup failure.  */

(but I think you could also look at your editor to see if there is
anything that's contributing to this, especially the inconsistency
- are you using fixed fonts?)

> +#define CancelIo dyn_CancelIo
> +
> +static BOOL WINAPI (*CancelIo) (HANDLE);

I would *personally* keep these two together, to make it clearer
that the comment above applies to both.  Just a thought, so don't
feel obligated to follow the suggestion if you don't agree.

>    /* Stop any pending selects.  */
> -  CancelIo ((HANDLE) _get_osfhandle (scb->fd));
> +  if (CancelIo)
> +    CancelIo ((HANDLE) _get_osfhandle (scb->fd));

Can you add a comment explaining why it's OK to not call CancelIo
if it is not defined? I think you were trying to get more info
about this on this list - whatever information that you might have
gleaned...

The patch is pre-approved once the comments are addressed.

-- 
Joel