This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.
- From: Eli Zaretskii <eliz at gnu dot org>
- To: Lerele <lerele at champenstudios dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 24 Feb 2007 14:07:05 +0200
- Subject: Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.
- References: <45DF7E27.10102@champenstudios.com>
- Reply-to: Eli Zaretskii <eliz at gnu dot org>
> Date: Sat, 24 Feb 2007 00:52:07 +0100
> From: Lerele <lerele@champenstudios.com>
>
> I've added remote interrupt support for gdbserver on win32.
> Also fixed gdbserver attach functionality, it wasn't even working at all.
Thanks!
> 2007-02-24 Leo Zayas
> * server.h, remote-utils.c: Expose input_interrupt through
> check_remote_input_interrupt_request for gdbserver.
> * win32-i386-low.c: Fix gdbserver attach support on win32.
> * win32-i386-low.c: Add remote interrupt support.
Please reformat the ChangeLog entries according to the GNU coding
standards (http://www.gnu.org/prep/standards/).
> +#ifdef USE_WIN32API
> +static int remote_desc=INVALID_SOCKET;
> +#else
> +static int remote_desc=-1;
> +#endif
I don't like using OS-dependent #define's where a functionality-based
#define can do the job. How about
+#ifndef INVALID_SOCKET
+#define INVALID_SOCKET -1
+#endif
and then use INVALID_SOCKET everywhere?
Also, this:
+static int remote_desc=INVALID_SOCKET;
is not according to GNU standards: you need spaces around `='.
> + /* We will be calling input_interrupt on win32 to check for remote
> interrupt,
Your mailer wraps lines, which will cause the Patch utility to fail to
apply the diffs. Please find a way to not wrap lines in the patch; if
nothing else helps, send it as a binary attachment.
> +#ifdef USE_WIN32API
> + if (remote_desc==INVALID_SOCKET)
> + return;
> +#else
> + if (remote_desc==-1)
> + return;
> +#endif
See above.
> -
> +
Please don't add excess whitespace. (There's a single blank on the
line you modified.)
> @@ -574,7 +584,7 @@
>
> FreeLibrary (kernel32);
>
> - return res;
> + return res? 0:-1;
I don't understand the need for this change. Can you explain?
child_continue does not promise to return exactly 1 when it fails,
only non-zero.