From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13614 invoked by alias); 26 Mar 2005 17:38:34 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 13474 invoked from network); 26 Mar 2005 17:38:25 -0000 Received: from unknown (HELO mail.codesourcery.com) (65.74.133.9) by sourceware.org with SMTP; 26 Mar 2005 17:38:25 -0000 Received: (qmail 2592 invoked from network); 26 Mar 2005 17:38:25 -0000 Received: from localhost (HELO ?192.168.0.5?) (mitchell@127.0.0.1) by mail.codesourcery.com with SMTP; 26 Mar 2005 17:38:25 -0000 Message-ID: <42459E07.9020201@codesourcery.com> Date: Sat, 26 Mar 2005 17:38:00 -0000 From: Mark Mitchell Organization: CodeSourcery, LLC User-Agent: Mozilla Thunderbird 0.9 (Windows/20041103) MIME-Version: 1.0 To: Mark Kettenis CC: gdb-patches@sources.redhat.com Subject: Re: PATCH: Windows sockets References: <200503260127.j2Q1R59a022152@sethra.codesourcery.com> <200503260845.j2Q8jlLC019248@elgar.sibelius.xs4all.nl> In-Reply-To: <200503260845.j2Q8jlLC019248@elgar.sibelius.xs4all.nl> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2005-03/txt/msg00343.txt.bz2 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