Follow-up on patch to ser-tcp.c

Follow-up on patch to ser-tcp.c

[Mark Mitchell]

Mark --

A while back, I posted a patch to change ser-tcp.c to support Windows, here:

http://sources.redhat.com/ml/gdb-patches/2005-03/msg00331.html

You had some objections:

1. The configury change to link with -lws2_23 on Windows.

2. The use of #ifdef WINAPI.

3. The asymmetry I introduced by adding the "read_prim" member to 
"struct serial" without a corresponding "write_prim" member.

4. Changes to support differences between BSD and Windows socket APIs.

5. Modifications to strerror.

I don't see a way around (1).  The question is really not whether the 
library is available; the question is whether we should use it.  On 
MinGW, we should; on Cygwin, we should not.

The impact of (2) can be mitigated by using __MINGW32__ instead of 
WINAPI.  That will avoid the defs.h change, and localize the changes to 
the actual conditional code.

In my local version of the patch, I've corrected (3); the symmetry is 
now complete.  I've also mitigated (4), in that I've determined that 
send/recv should work fine on all targets.  That means that we don't 
need an #ifdef to decide whether to use "read" or "recv"; we can just 
always use "recv", as that function is available both on Windows and 
elsewhere.

I can drop (5) from the patch, and we can come back to that later.

With the set of changes described here, would you be willing to consider 
the patch again?  If so, I will post the modified version.

Thanks,

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

Re: Follow-up on patch to ser-tcp.c

[Mark Kettenis]

   Date: Tue, 05 Apr 2005 11:05:24 -0700
   From: Mark Mitchell <mark@codesourcery.com>

   Mark --

   A while back, I posted a patch to change ser-tcp.c to support Windows, here:

   http://sources.redhat.com/ml/gdb-patches/2005-03/msg00331.html

Apologies for not getting back to you.

   You had some objections:

   1. The configury change to link with -lws2_23 on Windows.

   2. The use of #ifdef WINAPI.

   3. The asymmetry I introduced by adding the "read_prim" member to 
   "struct serial" without a corresponding "write_prim" member.

   4. Changes to support differences between BSD and Windows socket APIs.

   5. Modifications to strerror.

   I don't see a way around (1).  The question is really not whether the 
   library is available; the question is whether we should use it.  On 
   MinGW, we should; on Cygwin, we should not.

Not happy with it, but I suppose you're right.  But please read on.

   The impact of (2) can be mitigated by using __MINGW32__ instead of 
   WINAPI.  That will avoid the defs.h change, and localize the changes to 
   the actual conditional code.

Actually I think the WINAPI thingy is more descriptive.  In light of
the discussion above, I think there should be a single spot in
configure.ac where we decide whether we want to use the native Windows
API or the proper POSIX interfaces that Cygwin provides get used.
Some sort of "Windows but not Cygwin" would than add -lws2_23 and
define something like USE_WIN32API, and we'd use that define
everywhere where there is a choice between the native Windows API and
the proper POSIX interfaces.

   In my local version of the patch, I've corrected (3); the symmetry is 
   now complete.  I've also mitigated (4), in that I've determined that 
   send/recv should work fine on all targets.  That means that we don't 
   need an #ifdef to decide whether to use "read" or "recv"; we can just 
   always use "recv", as that function is available both on Windows and 
   elsewhere.

Great!  In this area we also have the ioctlsock v.s. ioctl issue.  Is
it possible to keep using the proper POSIX interfaces in the code and
#define ioctl ioctlsock in the USE_WIN32API case instead of the other
way around?  I think that makes it easier for non-Windows programmers
to understand the code.

   I can drop (5) from the patch, and we can come back to that later.

That'd be a good idea.  I get the feeling that errno values and
Windows OS errors are fundamentally different things.  Perhaps the
best way to solve this would be to translate the Windows OS errors
into the appropriate errno value ate the appropriate place.

   With the set of changes described here, would you be willing to consider 
   the patch again?  If so, I will post the modified version.

Please do so.

Mark

Re: Follow-up on patch to ser-tcp.c

[Mark Mitchell]

[-- Attachment #1: Type: text/plain, Size: 1757 bytes --]

Mark Kettenis wrote:
>    Date: Tue, 05 Apr 2005 11:05:24 -0700
>    From: Mark Mitchell <mark@codesourcery.com>
> 
>    Mark --
> 
>    A while back, I posted a patch to change ser-tcp.c to support Windows, here:
> 
>    http://sources.redhat.com/ml/gdb-patches/2005-03/msg00331.html
> 
> Apologies for not getting back to you.

I meant no criticism; I very much appreciate your help.

> Actually I think the WINAPI thingy is more descriptive.  In light of
> the discussion above, I think there should be a single spot in
> configure.ac where we decide whether we want to use the native Windows
> API or the proper POSIX interfaces that Cygwin provides get used.

OK; I've implemented that.

> Great!  In this area we also have the ioctlsock v.s. ioctl issue.  Is
> it possible to keep using the proper POSIX interfaces in the code and
> #define ioctl ioctlsock in the USE_WIN32API case instead of the other
> way around?  I think that makes it easier for non-Windows programmers
> to understand the code.

It's possible to "#define ioctl ioctlsocket" so that ioctl doesn't need 
to change.  However, I think it would be very confusing to do "#define 
close closesocket" because there's already a Windows runtime library 
function called "close" -- it just doesn't work on sockets.  So, in the 
version of the patch that's attached, we still do have a call to 
closesocket.

Here's the revised version.

One note: you might wonder why ser-base.c #include's <winsock2.h>.  The 
reason is that ser-base.c uses select, and struct timeval comes form 
that header on Windows.  We may eventually have to use something other 
than select on Windows, but we don't yet.

Is this version OK?

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

[-- Attachment #2: gdb.ser.patch --]
[-- Type: text/plain, Size: 25684 bytes --]

2005-04-06  Mark Mitchell  <mark@codesourcery.com>

	* configure.ac: On MinGW, define USE_WIN32API and link with
	-lws2_32.
	* ser-tcp.c (<winsock2.h>): Include, for Windows.
	(ETIMEDOUT): Define, for Windows.
	(ioctl): Likewise.
	(closesocket): Define, for POSIX.
	(net_open): Adjust for differences in socket functions between
	Windows and UNIX.
	(net_close): Likweise.
	(net_read_prim): New function.
	(net_write_prim): Likewise.
	(_initialize_ser_tcp): Initialize winsock.  Fill in read_prim and
	write_prim.
	* ser-unix.h (ser_unix_readcchar): Remove.
	(ser_unix_read_prim): Declare.
	(ser_unix_write_prim): Likewise.
	* ser-unix.c (generic_readchar): Move to ser-base.c.
	(ser_unix_wait_for): Likewise.
	(do_unix_readchar): Likewise.
	(ser_unix_readchar): Likewise.
	(_initialize_ser_hardwire): Initialize read_prim and write_prim.
	(ser_unix_read_prim): New function.
	(ser_unix_write_prim): Likewise.
	* ser-base.h (generic_readchar): Declare.
	(ser_base_readchar): Likewise.
	* ser-base.c (<winsock2.h>): Include, for windows.
	(fd_event): Use the read primitive specified by the serial
	interface.
	(ser_base_wait_for): Moved from ser-unix.c
	(do_ser_base_read_char): Likewise.
	(generic_readchar): Likewise.
	(ser_base_readchar): Likewise.
	(ser_base_write): Use the write primitive specified by the serial
	interface.
	* ser-pipe.c (_initialize_ser_pipe): Use ser_base_readchar, not
	ser_unix_readchar.  Initialize read_prim and write_prim.
	* serial.c (struct serial_ops): Add read_prim and write_prim.

*** /net/sethra/scratch/mitchell/src/gdb/gdb/configure.ac	Wed Apr  6 09:56:04 2005
--- configure.ac	Wed Apr  6 09:08:52 2005
*************** if test x$gdb_cv_os_cygwin = xyes; then
*** 1187,1192 ****
--- 1187,1204 ----
  	;;
      esac
  fi
+ 
+ # The ser-tcp.c module requires sockets.
+ case ${host} in
+   *mingw32*)
+     AC_DEFINE(USE_WIN32API, 1,
+               [Define if we should use the Windows API, instead of the 
+ 	       POSIX API.  On Windows, we use the Windows API when 
+ 	       building for MinGW, but the POSIX API when building 
+ 	       for Cygwin.])
+     WIN32LIBS="$WIN32LIBS -lws2_32"
+     ;;
+ esac	    
  AC_SUBST(WIN32LIBS)
  
  LIBGUI="../libgui/src/libgui.a"
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-base.c	Wed Apr  6 09:56:04 2005
--- ser-base.c	Wed Apr  6 09:59:39 2005
***************
*** 24,29 ****
--- 24,32 ----
  #include "serial.h"
  #include "ser-unix.h"
  #include "event-loop.h"
+ #ifdef USE_WIN32API
+ #include <winsock2.h>
+ #endif
  
  static timer_handler_func push_event;
  static handler_func fd_event;
*************** fd_event (int error, void *context)
*** 136,146 ****
           pull characters out of the buffer.  See also
           generic_readchar(). */
        int nr;
!       do
! 	{
! 	  nr = read (scb->fd, scb->buf, BUFSIZ);
! 	}
!       while (nr == -1 && errno == EINTR);
        if (nr == 0)
  	{
  	  scb->bufcnt = SERIAL_EOF;
--- 139,145 ----
           pull characters out of the buffer.  See also
           generic_readchar(). */
        int nr;
!       nr = scb->ops->read_prim (scb, BUFSIZ);
        if (nr == 0)
  	{
  	  scb->bufcnt = SERIAL_EOF;
*************** push_event (void *context)
*** 174,179 ****
--- 173,357 ----
    reschedule (scb);
  }
  
+ /* Wait for input on scb, with timeout seconds.  Returns 0 on success,
+    otherwise SERIAL_TIMEOUT or SERIAL_ERROR. */
+ 
+ static int
+ ser_base_wait_for (struct serial *scb, int timeout)
+ {
+   while (1)
+     {
+       int numfds;
+       struct timeval tv;
+       fd_set readfds, exceptfds;
+ 
+       /* NOTE: Some OS's can scramble the READFDS when the select()
+          call fails (ex the kernel with Red Hat 5.2).  Initialize all
+          arguments before each call. */
+ 
+       tv.tv_sec = timeout;
+       tv.tv_usec = 0;
+ 
+       FD_ZERO (&readfds);
+       FD_ZERO (&exceptfds);
+       FD_SET (scb->fd, &readfds);
+       FD_SET (scb->fd, &exceptfds);
+ 
+       if (timeout >= 0)
+ 	numfds = select (scb->fd + 1, &readfds, 0, &exceptfds, &tv);
+       else
+ 	numfds = select (scb->fd + 1, &readfds, 0, &exceptfds, 0);
+ 
+       if (numfds <= 0)
+ 	{
+ 	  if (numfds == 0)
+ 	    return SERIAL_TIMEOUT;
+ 	  else if (errno == EINTR)
+ 	    continue;
+ 	  else
+ 	    return SERIAL_ERROR;	/* Got an error from select or poll */
+ 	}
+ 
+       return 0;
+     }
+ }
+ 
+ /* Read a character with user-specified timeout.  TIMEOUT is number of seconds
+    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.  Returns
+    char if successful.  Returns -2 if timeout expired, EOF if line dropped
+    dead, or -3 for any other error (see errno in that case). */
+ 
+ static int
+ do_ser_base_readchar (struct serial *scb, int timeout)
+ {
+   int status;
+   int delta;
+ 
+   /* We have to be able to keep the GUI alive here, so we break the
+      original timeout into steps of 1 second, running the "keep the
+      GUI alive" hook each time through the loop.
+ 
+      Also, timeout = 0 means to poll, so we just set the delta to 0,
+      so we will only go through the loop once.  */
+ 
+   delta = (timeout == 0 ? 0 : 1);
+   while (1)
+     {
+ 
+       /* N.B. The UI may destroy our world (for instance by calling
+          remote_stop,) in which case we want to get out of here as
+          quickly as possible.  It is not safe to touch scb, since
+          someone else might have freed it.  The
+          deprecated_ui_loop_hook signals that we should exit by
+          returning 1.  */
+ 
+       if (deprecated_ui_loop_hook)
+ 	{
+ 	  if (deprecated_ui_loop_hook (0))
+ 	    return SERIAL_TIMEOUT;
+ 	}
+ 
+       status = ser_base_wait_for (scb, delta);
+       if (timeout > 0)
+         timeout -= delta;
+ 
+       /* If we got a character or an error back from wait_for, then we can 
+          break from the loop before the timeout is completed. */
+ 
+       if (status != SERIAL_TIMEOUT)
+ 	{
+ 	  break;
+ 	}
+ 
+       /* If we have exhausted the original timeout, then generate
+          a SERIAL_TIMEOUT, and pass it out of the loop. */
+ 
+       else if (timeout == 0)
+ 	{
+ 	  status = SERIAL_TIMEOUT;
+ 	  break;
+ 	}
+     }
+ 
+   if (status < 0)
+     return status;
+ 
+   status = scb->ops->read_prim (scb, BUFSIZ);
+ 
+   if (status <= 0)
+     {
+       if (status == 0)
+ 	return SERIAL_TIMEOUT;	/* 0 chars means timeout [may need to
+ 				   distinguish between EOF & timeouts
+ 				   someday] */
+       else
+ 	return SERIAL_ERROR;	/* Got an error from read */
+     }
+ 
+   scb->bufcnt = status;
+   scb->bufcnt--;
+   scb->bufp = scb->buf;
+   return *scb->bufp++;
+ }
+ 
+ /* Perform operations common to both old and new readchar. */
+ 
+ /* Return the next character from the input FIFO.  If the FIFO is
+    empty, call the SERIAL specific routine to try and read in more
+    characters.
+ 
+    Initially data from the input FIFO is returned (fd_event()
+    pre-reads the input into that FIFO.  Once that has been emptied,
+    further data is obtained by polling the input FD using the device
+    specific readchar() function.  Note: reschedule() is called after
+    every read.  This is because there is no guarentee that the lower
+    level fd_event() poll_event() code (which also calls reschedule())
+    will be called. */
+ 
+ int
+ generic_readchar (struct serial *scb, int timeout,
+ 		  int (do_readchar) (struct serial *scb, int timeout))
+ {
+   int ch;
+   if (scb->bufcnt > 0)
+     {
+       ch = *scb->bufp;
+       scb->bufcnt--;
+       scb->bufp++;
+     }
+   else if (scb->bufcnt < 0)
+     {
+       /* Some errors/eof are are sticky. */
+       ch = scb->bufcnt;
+     }
+   else
+     {
+       ch = do_readchar (scb, timeout);
+       if (ch < 0)
+ 	{
+ 	  switch ((enum serial_rc) ch)
+ 	    {
+ 	    case SERIAL_EOF:
+ 	    case SERIAL_ERROR:
+ 	      /* Make the error/eof stick. */
+ 	      scb->bufcnt = ch;
+ 	      break;
+ 	    case SERIAL_TIMEOUT:
+ 	      scb->bufcnt = 0;
+ 	      break;
+ 	    }
+ 	}
+     }
+   reschedule (scb);
+   return ch;
+ }
+ 
+ int
+ ser_base_readchar (struct serial *scb, int timeout)
+ {
+   return generic_readchar (scb, timeout, do_ser_base_readchar);
+ }
+ 
  int
  ser_base_write (struct serial *scb, const char *str, int len)
  {
*************** ser_base_write (struct serial *scb, cons
*** 181,187 ****
  
    while (len > 0)
      {
!       cc = write (scb->fd, str, len);
  
        if (cc < 0)
  	return 1;
--- 359,365 ----
  
    while (len > 0)
      {
!       cc = scb->ops->write_prim (scb, str, len); 
  
        if (cc < 0)
  	return 1;
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-pipe.c	Wed Apr  6 09:56:04 2005
--- ser-pipe.c	Mon Mar 28 10:41:42 2005
*************** _initialize_ser_pipe (void)
*** 144,150 ****
    ops->next = 0;
    ops->open = pipe_open;
    ops->close = pipe_close;
!   ops->readchar = ser_unix_readchar;
    ops->write = ser_base_write;
    ops->flush_output = ser_base_flush_output;
    ops->flush_input = ser_base_flush_input;
--- 144,150 ----
    ops->next = 0;
    ops->open = pipe_open;
    ops->close = pipe_close;
!   ops->readchar = ser_base_readchar;
    ops->write = ser_base_write;
    ops->flush_output = ser_base_flush_output;
    ops->flush_input = ser_base_flush_input;
*************** _initialize_ser_pipe (void)
*** 158,162 ****
--- 158,164 ----
    ops->setstopbits = ser_base_setstopbits;
    ops->drain_output = ser_base_drain_output;
    ops->async = ser_base_async;
+   ops->read_prim = ser_unix_read_prim;
+   ops->write_prim = ser_unix_write_prim;
    serial_add_interface (ops);
  }
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-tcp.c	Wed Apr  6 09:55:25 2005
--- ser-tcp.c	Wed Apr  6 09:16:51 2005
***************
*** 34,44 ****
--- 34,54 ----
  #endif
  
  #include <sys/time.h>
+ 
+ #ifdef USE_WIN32API
+ #include <winsock2.h>
+ #define ETIMEDOUT WSAETIMEDOUT
+ #define ioctl ioctlsocket
+ #else
  #include <netinet/in.h>
  #include <arpa/inet.h>
  #include <netdb.h>
  #include <sys/socket.h>
  #include <netinet/tcp.h>
+ /* We cannot define "close" to "closesocket" above because the
+    Windows API has "close", but it does not work on sockets.  */
+ #define closesocket close
+ #endif
  
  #include <signal.h>
  #include "gdb_string.h"
*************** net_open (struct serial *scb, const char
*** 62,67 ****
--- 72,82 ----
    int use_udp;
    struct hostent *hostent;
    struct sockaddr_in sockaddr;
+ #ifdef USE_WIN32API
+   u_long ioarg;
+ #else
+   int ioarg;
+ #endif
  
    use_udp = 0;
    if (strncmp (name, "udp:", 4) == 0)
*************** net_open (struct serial *scb, const char
*** 108,121 ****
  	  sizeof (struct in_addr));
  
    /* set socket nonblocking */
!   tmp = 1;
!   ioctl (scb->fd, FIONBIO, &tmp);
  
    /* Use Non-blocking connect.  connect() will return 0 if connected already. */
    n = connect (scb->fd, (struct sockaddr *) &sockaddr, sizeof (sockaddr));
  
!   if (n < 0 && errno != EINPROGRESS)
      {
        net_close (scb);
        return -1;
      }
--- 123,147 ----
  	  sizeof (struct in_addr));
  
    /* set socket nonblocking */
!   ioarg = 1;
!   ioctl (scb->fd, FIONBIO, &ioarg);
  
    /* Use Non-blocking connect.  connect() will return 0 if connected already. */
    n = connect (scb->fd, (struct sockaddr *) &sockaddr, sizeof (sockaddr));
  
!   if (n < 0
! #ifdef USE_WIN32API
!       /* Under Windows, calling "connect" with a non-blocking socket
! 	 results in WSAEWOULDBLOCK, not WSAEINPROGRESS.  */
!       && WSAGetLastError() != WSAEWOULDBLOCK
! #else
!       && errno != EINPROGRESS
! #endif
!       )
      {
+ #ifdef USE_WIN32API
+       errno = WSAGetLastError();
+ #endif
        net_close (scb);
        return -1;
      }
*************** net_open (struct serial *scb, const char
*** 165,171 ****
    {
      int res, err, len;
      len = sizeof(err);
!     res = getsockopt (scb->fd, SOL_SOCKET, SO_ERROR, &err, &len);
      if (res < 0 || err)
        {
  	if (err)
--- 191,201 ----
    {
      int res, err, len;
      len = sizeof(err);
!     /* On Windows, the fourth parameter to getsockopt is a "char *";
!        on UNIX systems it is generally "void *".  The cast to "void *"
!        is OK everywhere, since in C "void *" can be implicitly
!        converted to any pointer type.  */
!     res = getsockopt (scb->fd, SOL_SOCKET, SO_ERROR, (void *) &err, &len);
      if (res < 0 || err)
        {
  	if (err)
*************** net_open (struct serial *scb, const char
*** 176,183 ****
    } 
  
    /* turn off nonblocking */
!   tmp = 0;
!   ioctl (scb->fd, FIONBIO, &tmp);
  
    if (use_udp == 0)
      {
--- 206,213 ----
    } 
  
    /* turn off nonblocking */
!   ioarg = 0;
!   ioctl (scb->fd, FIONBIO, &ioarg);
  
    if (use_udp == 0)
      {
*************** net_close (struct serial *scb)
*** 202,221 ****
    if (scb->fd < 0)
      return;
  
!   close (scb->fd);
    scb->fd = -1;
  }
  
  void
  _initialize_ser_tcp (void)
  {
!   struct serial_ops *ops = XMALLOC (struct serial_ops);
    memset (ops, 0, sizeof (struct serial_ops));
    ops->name = "tcp";
    ops->next = 0;
    ops->open = net_open;
    ops->close = net_close;
!   ops->readchar = ser_unix_readchar;
    ops->write = ser_base_write;
    ops->flush_output = ser_base_flush_output;
    ops->flush_input = ser_base_flush_input;
--- 232,270 ----
    if (scb->fd < 0)
      return;
  
!   closesocket (scb->fd);
    scb->fd = -1;
  }
  
+ static int
+ net_read_prim (struct serial *scb, size_t count)
+ {
+   return recv (scb->fd, scb->buf, count, 0);
+ }
+ 
+ static int
+ net_write_prim (struct serial *scb, const void *buf, size_t count)
+ {
+   return send (scb->fd, buf, count, 0);
+ }
+ 
  void
  _initialize_ser_tcp (void)
  {
!   struct serial_ops *ops;
! #ifdef USE_WIN32API
!   WSADATA wsa_data;
!   if (WSAStartup (MAKEWORD (1, 0), &wsa_data) != 0)
!     /* WinSock is unavailable.  */
!     return;
! #endif
!   ops = XMALLOC (struct serial_ops);
    memset (ops, 0, sizeof (struct serial_ops));
    ops->name = "tcp";
    ops->next = 0;
    ops->open = net_open;
    ops->close = net_close;
!   ops->readchar = ser_base_readchar;
    ops->write = ser_base_write;
    ops->flush_output = ser_base_flush_output;
    ops->flush_input = ser_base_flush_input;
*************** _initialize_ser_tcp (void)
*** 229,233 ****
--- 278,284 ----
    ops->setstopbits = ser_base_setstopbits;
    ops->drain_output = ser_base_drain_output;
    ops->async = ser_base_async;
+   ops->read_prim = net_read_prim;
+   ops->write_prim = net_write_prim;
    serial_add_interface (ops);
  }
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-unix.c	Wed Apr  6 09:56:04 2005
--- ser-unix.c	Mon Mar 28 10:41:42 2005
*************** static void hardwire_raw (struct serial 
*** 70,78 ****
  static int wait_for (struct serial *scb, int timeout);
  static int hardwire_readchar (struct serial *scb, int timeout);
  static int do_hardwire_readchar (struct serial *scb, int timeout);
- static int generic_readchar (struct serial *scb, int timeout,
- 			     int (*do_readchar) (struct serial *scb,
- 						 int timeout));
  static int rate_to_code (int rate);
  static int hardwire_setbaudrate (struct serial *scb, int rate);
  static void hardwire_close (struct serial *scb);
--- 70,75 ----
*************** hardwire_raw (struct serial *scb)
*** 422,428 ****
   */
  
  /* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
!    ser_unix*() until the old TERMIOS/SGTTY/... timer code has been
     flushed. . */
  
  /* NOTE: cagney/1999-09-30: Much of the code below is dead.  The only
--- 419,425 ----
   */
  
  /* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
!    ser_base*() until the old TERMIOS/SGTTY/... timer code has been
     flushed. . */
  
  /* NOTE: cagney/1999-09-30: Much of the code below is dead.  The only
*************** wait_for (struct serial *scb, int timeou
*** 542,554 ****
     dropped dead, or SERIAL_ERROR for any other error (see errno in that case).  */
  
  /* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
!    ser_unix*() until the old TERMIOS/SGTTY/... timer code has been
     flushed. */
  
  /* NOTE: cagney/1999-09-16: This function is not identical to
!    ser_unix_readchar() as part of replacing it with ser_unix*()
     merging will be required - this code handles the case where read()
!    times out due to no data while ser_unix_readchar() doesn't expect
     that. */
  
  static int
--- 539,551 ----
     dropped dead, or SERIAL_ERROR for any other error (see errno in that case).  */
  
  /* FIXME: cagney/1999-09-16: Don't replace this with the equivalent
!    ser_base*() until the old TERMIOS/SGTTY/... timer code has been
     flushed. */
  
  /* NOTE: cagney/1999-09-16: This function is not identical to
!    ser_base_readchar() as part of replacing it with ser_base*()
     merging will be required - this code handles the case where read()
!    times out due to no data while ser_base_readchar() doesn't expect
     that. */
  
  static int
*************** hardwire_close (struct serial *scb)
*** 863,1053 ****
    close (scb->fd);
    scb->fd = -1;
  }
- 
  \f
- /* Wait for input on scb, with timeout seconds.  Returns 0 on success,
-    otherwise SERIAL_TIMEOUT or SERIAL_ERROR. */
- 
- static int
- ser_unix_wait_for (struct serial *scb, int timeout)
- {
-   while (1)
-     {
-       int numfds;
-       struct timeval tv;
-       fd_set readfds, exceptfds;
- 
-       /* NOTE: Some OS's can scramble the READFDS when the select()
-          call fails (ex the kernel with Red Hat 5.2).  Initialize all
-          arguments before each call. */
- 
-       tv.tv_sec = timeout;
-       tv.tv_usec = 0;
- 
-       FD_ZERO (&readfds);
-       FD_ZERO (&exceptfds);
-       FD_SET (scb->fd, &readfds);
-       FD_SET (scb->fd, &exceptfds);
- 
-       if (timeout >= 0)
- 	numfds = select (scb->fd + 1, &readfds, 0, &exceptfds, &tv);
-       else
- 	numfds = select (scb->fd + 1, &readfds, 0, &exceptfds, 0);
- 
-       if (numfds <= 0)
- 	{
- 	  if (numfds == 0)
- 	    return SERIAL_TIMEOUT;
- 	  else if (errno == EINTR)
- 	    continue;
- 	  else
- 	    return SERIAL_ERROR;	/* Got an error from select or poll */
- 	}
- 
-       return 0;
-     }
- }
- 
- /* Read a character with user-specified timeout.  TIMEOUT is number of seconds
-    to wait, or -1 to wait forever.  Use timeout of 0 to effect a poll.  Returns
-    char if successful.  Returns -2 if timeout expired, EOF if line dropped
-    dead, or -3 for any other error (see errno in that case). */
- 
- static int
- do_unix_readchar (struct serial *scb, int timeout)
- {
-   int status;
-   int delta;
- 
-   /* We have to be able to keep the GUI alive here, so we break the
-      original timeout into steps of 1 second, running the "keep the
-      GUI alive" hook each time through the loop.
- 
-      Also, timeout = 0 means to poll, so we just set the delta to 0,
-      so we will only go through the loop once.  */
- 
-   delta = (timeout == 0 ? 0 : 1);
-   while (1)
-     {
- 
-       /* N.B. The UI may destroy our world (for instance by calling
-          remote_stop,) in which case we want to get out of here as
-          quickly as possible.  It is not safe to touch scb, since
-          someone else might have freed it.  The
-          deprecated_ui_loop_hook signals that we should exit by
-          returning 1.  */
- 
-       if (deprecated_ui_loop_hook)
- 	{
- 	  if (deprecated_ui_loop_hook (0))
- 	    return SERIAL_TIMEOUT;
- 	}
- 
-       status = ser_unix_wait_for (scb, delta);
-       if (timeout > 0)
-         timeout -= delta;
- 
-       /* If we got a character or an error back from wait_for, then we can 
-          break from the loop before the timeout is completed. */
- 
-       if (status != SERIAL_TIMEOUT)
- 	{
- 	  break;
- 	}
- 
-       /* If we have exhausted the original timeout, then generate
-          a SERIAL_TIMEOUT, and pass it out of the loop. */
- 
-       else if (timeout == 0)
- 	{
- 	  status = SERIAL_TIMEOUT;
- 	  break;
- 	}
-     }
- 
-   if (status < 0)
-     return status;
- 
-   while (1)
-     {
-       status = read (scb->fd, scb->buf, BUFSIZ);
-       if (status != -1 || errno != EINTR)
- 	break;
-     }
- 
-   if (status <= 0)
-     {
-       if (status == 0)
- 	return SERIAL_TIMEOUT;	/* 0 chars means timeout [may need to
- 				   distinguish between EOF & timeouts
- 				   someday] */
-       else
- 	return SERIAL_ERROR;	/* Got an error from read */
-     }
- 
-   scb->bufcnt = status;
-   scb->bufcnt--;
-   scb->bufp = scb->buf;
-   return *scb->bufp++;
- }
- 
- /* Perform operations common to both old and new readchar. */
- 
- /* Return the next character from the input FIFO.  If the FIFO is
-    empty, call the SERIAL specific routine to try and read in more
-    characters.
- 
-    Initially data from the input FIFO is returned (fd_event()
-    pre-reads the input into that FIFO.  Once that has been emptied,
-    further data is obtained by polling the input FD using the device
-    specific readchar() function.  Note: reschedule() is called after
-    every read.  This is because there is no guarentee that the lower
-    level fd_event() poll_event() code (which also calls reschedule())
-    will be called. */
- 
- static int
- generic_readchar (struct serial *scb, int timeout,
- 		  int (do_readchar) (struct serial *scb, int timeout))
- {
-   int ch;
-   if (scb->bufcnt > 0)
-     {
-       ch = *scb->bufp;
-       scb->bufcnt--;
-       scb->bufp++;
-     }
-   else if (scb->bufcnt < 0)
-     {
-       /* Some errors/eof are are sticky. */
-       ch = scb->bufcnt;
-     }
-   else
-     {
-       ch = do_readchar (scb, timeout);
-       if (ch < 0)
- 	{
- 	  switch ((enum serial_rc) ch)
- 	    {
- 	    case SERIAL_EOF:
- 	    case SERIAL_ERROR:
- 	      /* Make the error/eof stick. */
- 	      scb->bufcnt = ch;
- 	      break;
- 	    case SERIAL_TIMEOUT:
- 	      scb->bufcnt = 0;
- 	      break;
- 	    }
- 	}
-     }
-   reschedule (scb);
-   return ch;
- }
- 
- int
- ser_unix_readchar (struct serial *scb, int timeout)
- {
-   return generic_readchar (scb, timeout, do_unix_readchar);
- }
  \f
  void
  _initialize_ser_hardwire (void)
--- 860,866 ----
*************** _initialize_ser_hardwire (void)
*** 1058,1064 ****
    ops->next = 0;
    ops->open = hardwire_open;
    ops->close = hardwire_close;
!   /* FIXME: Don't replace this with the equivalent ser_unix*() until
       the old TERMIOS/SGTTY/... timer code has been flushed. cagney
       1999-09-16. */
    ops->readchar = hardwire_readchar;
--- 871,877 ----
    ops->next = 0;
    ops->open = hardwire_open;
    ops->close = hardwire_close;
!   /* FIXME: Don't replace this with the equivalent ser_base*() until
       the old TERMIOS/SGTTY/... timer code has been flushed. cagney
       1999-09-16. */
    ops->readchar = hardwire_readchar;
*************** _initialize_ser_hardwire (void)
*** 1075,1079 ****
--- 888,916 ----
    ops->setstopbits = hardwire_setstopbits;
    ops->drain_output = hardwire_drain_output;
    ops->async = ser_base_async;
+   ops->read_prim = ser_unix_read_prim;
+   ops->write_prim = ser_unix_write_prim;
    serial_add_interface (ops);
  }
+ 
+ int
+ ser_unix_read_prim (struct serial *scb, size_t count)
+ {
+   int status;
+ 
+   while (1)
+     {
+       status = read (scb->fd, scb->buf, count);
+       if (status != -1 || errno != EINTR)
+ 	break;
+     }
+   return status;
+ }
+ 
+ int
+ ser_unix_write_prim (struct serial *scb, const void *buf, size_t len)
+ {
+   /* ??? Historically, GDB has not retried calls to "write" that
+      result in EINTR.  */
+   return write (scb->fd, buf, len);
+ }
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-base.h	Wed Apr  6 09:56:04 2005
--- ser-base.h	Mon Mar 28 10:41:42 2005
***************
*** 25,30 ****
--- 25,33 ----
  struct serial;
  struct ui_file;
  
+ extern int generic_readchar (struct serial *scb, int timeout,
+ 			     int (*do_readchar) (struct serial *scb,
+ 						 int timeout));
  extern void reschedule (struct serial *scb);
  extern int ser_base_flush_output (struct serial *scb);
  extern int ser_base_flush_input (struct serial *scb);
*************** extern int ser_base_drain_output (struct
*** 46,50 ****
--- 49,54 ----
  extern int ser_base_write (struct serial *scb, const char *str, int len);
  
  extern void ser_base_async (struct serial *scb, int async_p);
+ extern int ser_base_readchar (struct serial *scb, int timeout);
  
  #endif
*** /net/sethra/scratch/mitchell/src/gdb/gdb/ser-unix.h	Wed Apr  6 09:56:04 2005
--- ser-unix.h	Mon Mar 28 10:41:42 2005
***************
*** 22,27 ****
  #ifndef SER_UNIX_H
  #define SER_UNIX_H
  
! extern int ser_unix_readchar (struct serial *scb, int timeout);
  
  #endif
--- 22,29 ----
  #ifndef SER_UNIX_H
  #define SER_UNIX_H
  
! extern int ser_unix_read_prim (struct serial *scb, size_t count);
! extern int ser_unix_write_prim (struct serial *scb, const void *buf, 
! 				size_t count);
  
  #endif
*** /net/sethra/scratch/mitchell/src/gdb/gdb/serial.h	Wed Apr  6 09:56:04 2005
--- serial.h	Mon Mar 28 10:41:42 2005
*************** struct serial_ops
*** 232,237 ****
--- 232,243 ----
         the specified function when ever there is something
         interesting.  */
      void (*async) (struct serial *scb, int async_p);
+     /* Perform a low-level read operation, reading (at most) COUNT
+        bytes into SCB->BUF.  */
+     int (*read_prim)(struct serial *scb, size_t count);
+     /* Perform a low-level write operation, writing (at most) COUNT
+        bytes from BUF.  */
+     int (*write_prim)(struct serial *scb, const void *buf, size_t count);
    };
  
  /* Add a new serial interface to the interface list */

Re: Follow-up on patch to ser-tcp.c

[Kai Henningsen]

mark.kettenis@xs4all.nl (Mark Kettenis)  wrote on 06.04.05 in <200504060739.j367dlM3000821@elgar.sibelius.xs4all.nl>:

> Actually I think the WINAPI thingy is more descriptive.  In light of
> the discussion above, I think there should be a single spot in
> configure.ac where we decide whether we want to use the native Windows
> API or the proper POSIX interfaces that Cygwin provides get used.
> Some sort of "Windows but not Cygwin" would than add -lws2_23 and
> define something like USE_WIN32API, and we'd use that define
> everywhere where there is a choice between the native Windows API and
> the proper POSIX interfaces.

I seem to recall that autoconf does know something about that difference.

> Great!  In this area we also have the ioctlsock v.s. ioctl issue.  Is
> it possible to keep using the proper POSIX interfaces in the code and
> #define ioctl ioctlsock in the USE_WIN32API case instead of the other
> way around?  I think that makes it easier for non-Windows programmers
> to understand the code.

The whole point is that Windows has two different ioctl functions, whereas  
POSIX only has one. Now if you never need to use the non-socket ioctl on  
Windows, that would be possible - otherwise ...

MfG Kai