From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-47776-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 27818 invoked by alias); 24 Feb 2007 12:07:14 -0000
Received: (qmail 27509 invoked by uid 22791); 24 Feb 2007 12:07:13 -0000
X-Spam-Check-By: sourceware.org
Received: from nitzan.inter.net.il (HELO nitzan.inter.net.il) (213.8.233.22)     by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 24 Feb 2007 12:07:07 +0000
Received: from HOME-C4E4A596F7 (IGLD-80-230-20-14.inter.net.il [80.230.20.14]) 	by nitzan.inter.net.il (MOS 3.7.3a-GA) 	with ESMTP id GCV75272 (AUTH halo1); 	Sat, 24 Feb 2007 14:07:00 +0200 (IST)
Date: Sat, 24 Feb 2007 12:07:00 -0000
Message-Id: <ulkinvjx2.fsf@gnu.org>
From: Eli Zaretskii <eliz@gnu.org>
To: Lerele <lerele@champenstudios.com>
CC: gdb-patches@sourceware.org
In-reply-to: <45DF7E27.10102@champenstudios.com> (message from Lerele on Sat, 	24 Feb 2007 00:52:07 +0100)
Subject: Re: [Patch] Win32 gdbserver new interrupt support, and attach to process  fix.
Reply-to: Eli Zaretskii <eliz@gnu.org>
References:  <45DF7E27.10102@champenstudios.com>
X-IsSubscribed: yes
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
X-SW-Source: 2007-02/txt/msg00296.txt.bz2

> 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.