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: Lerele <lerele at champenstudios dot com>
- To: Eli Zaretskii <eliz at gnu dot org>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 24 Feb 2007 14:02:37 +0100
- Subject: Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix.
- References: <45DF7E27.10102@champenstudios.com> <ulkinvjx2.fsf@gnu.org>
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;