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

[Eli Zaretskii <eliz@gnu.org>]

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