This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] mingw port: Allow use of cvs GDB on Windows 95
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Pierre Muller <pierre dot muller at ics-cnrs dot unistra dot fr>
- Cc: gdb-patches at sourceware dot org
- Date: Mon, 21 Feb 2011 14:21:51 +0400
- Subject: Re: [RFA] mingw port: Allow use of cvs GDB on Windows 95
- References: <001201cbc46c$5b95b100$12c11300$@muller@ics-cnrs.unistra.fr> <000001cbc473$6ef9d120$4ced7360$@muller@ics-cnrs.unistra.fr> <002401cbc574$a8d990e0$fa8cb2a0$@muller@ics-cnrs.unistra.fr>
> 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