Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.

[Eli Zaretskii <eliz@gnu.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.