From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18343 invoked by alias); 24 Feb 2007 13:03:07 -0000 Received: (qmail 18333 invoked by uid 22791); 24 Feb 2007 13:03:06 -0000 X-Spam-Check-By: sourceware.org Received: from 164.Red-80-36-45.staticIP.rima-tde.net (HELO jupiter.champenstudios.com) (80.36.45.164) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 24 Feb 2007 13:02:59 +0000 Received: from [192.168.1.132] ([192.168.1.132]) by jupiter.champenstudios.com (8.13.5/8.13.5) with ESMTP id l1OCx50x005566; Sat, 24 Feb 2007 13:59:06 +0100 Message-ID: <45E0376D.9010605@champenstudios.com> Date: Sat, 24 Feb 2007 13:03:00 -0000 From: Lerele User-Agent: Mozilla Thunderbird 0.9 (Windows/20041103) MIME-Version: 1.0 To: Eli Zaretskii CC: gdb-patches@sourceware.org Subject: Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix. References: <45DF7E27.10102@champenstudios.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-02/txt/msg00297.txt.bz2 Oops. This was my first patch submission directly to the list (last one I sent directly to gdbserver maintainer), something had to go wrong, sorry. >>+#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? Since this define will be in non OS specific files, wouldn't it "hurt" to be reading that portable code with OS specific embedded words? Personally I'd prefer something like: #if USE_WIN32API #define BADSOCKET INVALID_SOCKET #else #define BADSOCKET -1 #endif However I believe it's natural to use -1 directly to check for bad socket in non-win32 code, so maybe using INVALID_SOCKET or BADSOCKET everywhere would make code uglier. Tell me what you prefer. > > >>@@ -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. 'res' as it is indicates success if it's != 0 (it's a Win32 BOOL), however the attach to process function should indicate success returning 0, or -1 if it cannot attach (see server.c 'attach_inferior' function). That's what this line does. Is it necessary to add a source code comment here to explain this? Or change the line; maybe this line would be clearer: return res != FALSE? 0:-1;