This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Windows sockets



Thanks for the quick review.


Here is a sumary of the changes in this patch:

1. Link with -lws2_32 on MinGW so we can use sockets.

Can't you just use a standard autoconf check to check for that library
instead of hardcoding it to be linked in on MinGW?

I could do that -- but I didn't because there are Windows configurations that do not want to use Winsock. For example, we don't want to link in this library on Cygwin; we just want to let the Cygwin DLL take care of things.


   2. In defs.h, define WINAPI so that source files know whether they are
      supposed to use the Windows API.

This is bad.  Yet another #ifdef WINDHOOS when we really are trying to
get rid of those.

This is the same problem as above; I could autoconf things for Windows, but those autoconf tests would fire under Cygwin as well.


   3. Move ser_unix_readchar and friends (which were only barely
      UNIX-specific) into ser-base.c, renaming appropriately.

Fair enough.

   4. Add a new member (read_prim) to struct serial_ops, and use it from
      ser_base_readchar.  The reason for this is that sockets must be
      read with "recv" on Windows; plain "read" does not work.  So, we
      need a way to indicate which low-level primitive to use to read
      from a file descriptor.

But write(2) does work and you don't need to use send(2). Oh you do
but things are not quite symmetric. Could you fix that?

I'm not sure what you mean. Do you mean that you would like to create write_prim, and use that from the high-level serial "write" function? Sure, I can do that.


> Isn't it a
better idea to change the #ifdef WINAPI in a #ifdef HAVE_RECV, add an
autoconf check for recv(2), and always prefer it over read(2) if
available for reading from a socket?

I was confused about that. I thought that using recv would not work as well as read on UNIX, but I've done some extra research, and I think send/recv should work fine everywhere. Please consider the change you suggest made.


   5. Make a handful of minor changes to ter-tcp.c to account for
      differences in the BSD and Windows sockets APIs.

You mean that Windows doesn't have the proper BSD socket API even
though Microsoft stole^Wused the BSD TCP stack?

Yes, there are a few minor differences.


I really think these changes are too pervasive and that you really
need to create a ser-win32.c that has all the Windows-specific cruft
in it.

I can do that.


However, after making the changes you suggest above, there will only be the following Windows-specific changes:

1. Macro definitions at top-of-file to unify the Windows/UNIX spellings of ETIMEDOUT, closesocket, and ioctlsocket.

2. The change top net_open to use a "u_long *" argument as the last argument to ioctl, rather than an "int *". (We could avoid the #ifdef with a typedef at the top of the file, if that's better than the way I have it now.)

3. The change to net_open to use WSAGetLastError() instead of errno on Windows.

4. The change to _initialize_ser_tcp to initialize the sockets library.

To me, it seems a shame to duplicate the entire file (in particular, net_open, which is the meat of the file) on account of these differences. That function is mostly platform independent, including parsing the host/port specification, setting up the socket, connecting the socket, interacting with the UI during the connection delay, etc.

Would you please tell me definitively that you want this copied to another file? I'm perfectly willing to do it, but I want to make sure that it's really the right thing before I do. Or, perhaps you mean that you want hooks inserted at all the right places, and overridden in ser-win32.c? I can also do that, but I think it will make the code harder to read. Please let me know.

6. Tweak safe_strerror to deal with Windows sockets error codes.

I'm defenitely not thrilled by this tweak.  You're only changing
"undocumented" into "winsock".  I presume it helps with debugging this
stuff, but is it really worth the clutter it adds?

I think so, yes. Windows strerror never returns NULL. For unknown values, it returns "Unknown error" with no indication of *which* unknown error. These errors are presented to users, so, if for example, a socket cannot be connected because the user entered the wrong port, or gdbserver is not running, the user would only see "Unknown error". I agree that, from a user-experience point of view, "winsock error 12345" is not all that helpful -- but at least there is *some* method for figuring out what went wrong.


Thanks,

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]