From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gdb-patches-return-47778-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Received: (qmail 19545 invoked by alias); 24 Feb 2007 14:07:12 -0000
Received: (qmail 19413 invoked by uid 22791); 24 Feb 2007 14:07:11 -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 14:07:05 +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 GCW12941 (AUTH halo1); 	Sat, 24 Feb 2007 16:06:43 +0200 (IST)
Date: Sat, 24 Feb 2007 14:07:00 -0000
Message-Id: <uejofvedj.fsf@gnu.org>
From: Eli Zaretskii <eliz@gnu.org>
To: Lerele <lerele@champenstudios.com>
CC: gdb-patches@sourceware.org
In-reply-to: <45E0376D.9010605@champenstudios.com> (message from Lerele on 	Sat, 24 Feb 2007 14:02:37 +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> <ulkinvjx2.fsf@gnu.org> <45E0376D.9010605@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/msg00298.txt.bz2

> Date: Sat, 24 Feb 2007 14:02:37 +0100
> From: Lerele <lerele@champenstudios.com>
> CC: gdb-patches@sourceware.org
> 
> >>+#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?
> >  
> >
> Isn't INVALID_SOCKET just an OS specific define?

It is defined on some systems, but not on others.  However, it is
(IMO) cleaner to use the defined symbol than to use the name of the OS
or an OS-specific API, because if tomorrow some other supported
platform will define INVALID_SOCKET, the code I suggested will work
without any changes, while yours will require to add that other
platform's name to the #ifdef.

> >>@@ -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.
> >
> >
> >  
> >
> 
> That should actually be the simple fix for attach to process (function 
> win32_attach). Do you see that line in child_continue function? Strange. 
> It should be the last line in win32_attach.

Sorry, you are right, I was looking at the wrong function.