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]

Re: gdb-5.0 and select()


[list changed to gdb-patches]

> Hi.  I have found what I think is a bug in gdb/ser-unix.c in
> gdb-5.0.  The functions wait_for() and ser_unix_wait_for() do
> FD_ZERO() and FD_SET() before a loop on select() that either 
> returns or continues based on the return value of select.  If 
> select returns -1 with EINTR, we continue and loop on select again.
> 
> The behavior of select() in the EINTR case seems to be platform 
> dependant, specifically regarding the contents of the descriptor
> sets after the EINTR.
> 
> man select:
> On sparc-sun-solaris2.6:
> 
>         On failure, the objects pointed to by the  readfs,  writefs,
>         and  errorfds  arguments  are  not modified.
> 
> 
> On linux Redhat5.2:
> 
>         On error, -1 is returned, and errno is set appropriately; the
>         sets and timeout become undefined, so do not rely on their
>         contents after an error.
> 
> 
> On our new OS (QNX RTP/Neutrino):
> 
>         On return, the select() function replaces the given descriptor 
>         sets with subsets consisting of those descriptors that are ready 
>         for the requested operation. 
> 
> 
> Three different behaviors.  In my case, I placed the FD_ZERO() and
> FD_SET() calls inside the while loop above the select(), and this 
> fixed it for me.  Should this be the default?  I do not see how it
> would break things for other platforms.

Yes, I agree, the FD_SET() should be moved to inside of the for loop. 
After some testing I'll apply the attatched patch.  Can you give it a
wirl?

	Andrew

PS: Which Linux kernel (rather than distro).



Hi.  I have found what I think is a bug in gdb/ser-unix.c in
gdb-5.0.  The functions wait_for() and ser_unix_wait_for() do
FD_ZERO() and FD_SET() before a loop on select() that either 
returns or continues based on the return value of select.  If 
select returns -1 with EINTR, we continue and loop on select again.

The behavior of select() in the EINTR case seems to be platform 
dependant, specifically regarding the contents of the descriptor
sets after the EINTR.

man select:
On sparc-sun-solaris2.6:

	On failure, the objects pointed to by the  readfs,  writefs,
	and  errorfds  arguments  are  not modified.


On linux Redhat5.2:

	On error, -1 is returned, and errno is set appropriately; the
	sets and timeout become undefined, so do not rely on their
	contents after an error.


On our new OS (QNX RTP/Neutrino):

	On return, the select() function replaces the given descriptor 
	sets with subsets consisting of those descriptors that are ready 
	for the requested operation. 


Three different behaviors.  In my case, I placed the FD_ZERO() and
FD_SET() calls inside the while loop above the select(), and this 
fixed it for me.  Should this be the default?  I do not see how it
would break things for other platforms.

ie:  (for ser_unix_wait_for and similarly for wait_for)

int
ser_unix_wait_for (serial_t scb, int timeout)
{
  int numfds;
  struct timeval tv;
  fd_set readfds, exceptfds;

  FD_ZERO (&readfds);
  FD_ZERO (&exceptfds);

  tv.tv_sec = timeout;
  tv.tv_usec = 0;

  FD_SET (scb->fd, &readfds);
  FD_SET (scb->fd, &exceptfds);

  while (1)
    {
      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;
    }
}

becomes

int
ser_unix_wait_for (serial_t scb, int timeout)
{
  int numfds;
  struct timeval tv;
  fd_set readfds, exceptfds;

  tv.tv_sec = timeout;
  tv.tv_usec = 0;

  while (1)
    {
      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;
    }
}

Thanks.
GP

---------------
Graeme Peterson
QNX Tools Group
gp@qnx.com


_______________________________________________
Bug-gdb mailing list
Bug-gdb@gnu.org
http://mail.gnu.org/mailman/listinfo/bug-gdb



Tue Nov 21 03:20:18 2000  Andrew Cagney  <cagney@b1.cygnus.com>

	* ser-unix.c (wait_for): Initialize the FD_SET before every select
 	call.
	(ser_unix_wait_for): Ditto.

Index: ser-unix.c
===================================================================
RCS file: /cvs/src/src/gdb/ser-unix.c,v
retrieving revision 1.8
diff -p -r1.8 ser-unix.c
*** ser-unix.c	2000/10/30 21:50:57	1.8
--- ser-unix.c	2000/11/20 16:26:37
*************** static int
*** 435,471 ****
  wait_for (serial_t scb, int timeout)
  {
  #ifdef HAVE_SGTTY
!   {
!     struct timeval tv;
!     fd_set readfds;
! 
!     FD_ZERO (&readfds);
! 
!     tv.tv_sec = timeout;
!     tv.tv_usec = 0;
! 
!     FD_SET (scb->fd, &readfds);
! 
!     while (1)
!       {
! 	int numfds;
! 
! 	if (timeout >= 0)
! 	  numfds = select (scb->fd + 1, &readfds, 0, 0, &tv);
  	else
! 	  numfds = select (scb->fd + 1, &readfds, 0, 0, 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;
!       }
!   }
  #endif /* HAVE_SGTTY */
  
  #if defined HAVE_TERMIO || defined HAVE_TERMIOS
--- 435,471 ----
  wait_for (serial_t scb, int timeout)
  {
  #ifdef HAVE_SGTTY
!   while (1)
!     {
!       struct timeval tv;
!       fd_set readfds;
!       int numfds;
! 
!       /* 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_SET (scb->fd, &readfds);
! 
!       if (timeout >= 0)
! 	numfds = select (scb->fd + 1, &readfds, 0, 0, &tv);
!       else
! 	numfds = select (scb->fd + 1, &readfds, 0, 0, 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;
!     }
  #endif /* HAVE_SGTTY */
  
  #if defined HAVE_TERMIO || defined HAVE_TERMIOS
*************** ser_unix_nop_raw (serial_t scb)
*** 858,878 ****
  int
  ser_unix_wait_for (serial_t scb, int timeout)
  {
-   int numfds;
-   struct timeval tv;
-   fd_set readfds, exceptfds;
- 
-   FD_ZERO (&readfds);
-   FD_ZERO (&exceptfds);
- 
-   tv.tv_sec = timeout;
-   tv.tv_usec = 0;
- 
-   FD_SET (scb->fd, &readfds);
-   FD_SET (scb->fd, &exceptfds);
- 
    while (1)
      {
        if (timeout >= 0)
  	numfds = select (scb->fd + 1, &readfds, 0, &exceptfds, &tv);
        else
--- 858,881 ----
  int
  ser_unix_wait_for (serial_t 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

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