Fix use of the close system call.

When close returns, it guarantees that the file descriptor given will
be closed, even if it fails.  (The documentation is extremely poor,
but this is what every operating system does.)  Consequently, it is a
bug to retry close.  Every use now either ignores the return value of
close or takes a specific action based on it; it is incorrect to use
STD_VOID_SYSTEM_CALL with close.

Some calls that ignore the return value really need to check for
EINTR and deliver pending interrupts, but I'll get to that later.

diff --git a/src/microcode/ux.c b/src/microcode/ux.c
index 1898300093fc93f82b16aeca3ba48b53d1537d16..497d133749c7a321d96bfeb8047877eac1affa1c 100644 (file)
--- a/src/microcode/ux.c
+++ b/src/microcode/ux.c
@@ -206,8 +206,8 @@ UX_setsid (void)
   int fd = (UX_open ("/dev/tty", O_RDWR, 0));
   if (fd >= 0)
     {
-      UX_ioctl (fd, TIOCNOTTY, 0);
-      UX_close (fd);
+      (void) UX_ioctl (fd, TIOCNOTTY, 0);
+      (void) UX_close (fd);
     }
 #endif
   return (setpgrp (0, 0));
@@ -366,7 +366,7 @@ int
 UX_dup2 (int fd, int fd2)
 {
   if (fd != fd2)
-    UX_close (fd2);
+    (void) UX_close (fd2);
   {
     int result = (UX_fcntl (fd, F_DUPFD, fd2));
     if ((result < 0) && (errno == EINVAL))
@@ -634,19 +634,21 @@ UX_closefrom (int fd)
   return (UX_fcntl (fd, F_CLOSEM));
 #elif ((defined (HAVE_FCNTL)) && (defined (F_MAXFD)))
   int max_fd = (UX_fcntl ((-1), F_MAXFD));
-  int status;
+  int status = 0, error = 0;
   if (max_fd < 0) return (max_fd);
   while (fd <= max_fd)
-    if (((status = (UX_close (fd++))) < 0) && (errno != EBADF))
-      return (status);
-  return (0);
+    if (((UX_close (fd++)) < 0) && (errno != EBADF))
+      status = (-1), error = errno;
+  errno = error;
+  return (status);
 #else
   int fd_limit = (UX_SC_OPEN_MAX ());
-  int status;
+  int status = 0, error = 0;
   while (fd < fd_limit)
-    if (((status = (UX_close (fd++))) < 0) && (errno != EBADF))
-      return (status);
-  return (0);
+    if (((UX_close (fd++)) < 0) && (errno != EBADF))
+      status = (-1), error = errno;
+  errno = error;
+  return (status);
 #endif
 }
 

diff --git a/src/microcode/uxfile.c b/src/microcode/uxfile.c
index a9b2504e646076a6df1ebaa31166cec7f55920e5..02679a69e92793ea3238100bfdff01005877a6d8 100644 (file)
--- a/src/microcode/uxfile.c
+++ b/src/microcode/uxfile.c
@@ -77,7 +77,8 @@ open_file (const char * filename, int oflag)
   if ((SLAVE_PTY_P (filename)) && (!UX_setup_slave_pty (fd)))
     {
       int xerrno = errno;
-      UX_close (fd);
+      /* FIXME: Need to check for EINTR.  */
+      (void) UX_close (fd);
       error_system_call (xerrno, syscall_open);
     }
 #endif

diff --git a/src/microcode/uxfs.c b/src/microcode/uxfs.c
index 46802430a030637e1c515f82434783d68a97d224..18cdbec5332f663c945c55d2d2ade8e082bf4019 100644 (file)
--- a/src/microcode/uxfs.c
+++ b/src/microcode/uxfs.c
@@ -479,9 +479,10 @@ OS_file_touch (const char * filename)
        transaction_commit ();
 #else
        transaction_commit ();
+       /* FIXME: Need to check for EINTR.  */
        fd = (UX_open (filename, (O_WRONLY | O_TRUNC), MODE_REG));
        if (fd >= 0)
-         STD_VOID_SYSTEM_CALL (syscall_close, (UX_close (fd)));
+         (void) UX_close (fd);
 #endif
        return (0);
       }
@@ -504,7 +505,7 @@ OS_file_touch (const char * filename)
 static void
 protect_fd_close (void * ap)
 {
-  UX_close (* ((int *) ap));
+  (void) UX_close (* ((int *) ap));
 }
 
 static void

diff --git a/src/microcode/uxio.c b/src/microcode/uxio.c
index df221168f1915a0163da61e7c04ab9524fd2b97f..353620709e48825e428cfbc32559a316760d4bb2 100644 (file)
--- a/src/microcode/uxio.c
+++ b/src/microcode/uxio.c
@@ -124,8 +124,12 @@ OS_channel_close (Tchannel channel)
 {
   if (! (CHANNEL_INTERNAL (channel)))
     {
-      STD_VOID_SYSTEM_CALL
-       (syscall_close, (UX_close (CHANNEL_DESCRIPTOR (channel))));
+      if (0 > (UX_close (CHANNEL_DESCRIPTOR (channel))))
+       switch (errno)
+         {
+         case EINTR:   deliver_pending_interrupts ();                  break;
+         case EBADF:   error_system_call (errno, syscall_close);       break;
+         }
       MARK_CHANNEL_CLOSED (channel);
     }
 }
@@ -135,7 +139,7 @@ OS_channel_close_noerror (Tchannel channel)
 {
   if (! (CHANNEL_INTERNAL (channel)))
     {
-      UX_close (CHANNEL_DESCRIPTOR (channel));
+      (void) UX_close (CHANNEL_DESCRIPTOR (channel));
       MARK_CHANNEL_CLOSED (channel);
     }
 }

diff --git a/src/microcode/uxproc.c b/src/microcode/uxproc.c
index ffe7f7d1e2c5b729b686626153e0885f8642a27e..c52441c41c30ac56d06ee1f20f87f1f047f4f033 100644 (file)
--- a/src/microcode/uxproc.c
+++ b/src/microcode/uxproc.c
@@ -416,15 +416,14 @@ OS_make_subprocess (const char * filename,
   }
 
   /* Close all file descriptors not used by the child.  */
-  /* FIXME: Handle EINTR?  */
   if (channel_in_type == process_channel_type_none)
-    if ((UX_close (STDIN_FILENO)) < 0) goto kill_child;
+    (void) UX_close (STDIN_FILENO);
   if (channel_out_type == process_channel_type_none)
-    if ((UX_close (STDOUT_FILENO)) < 0) goto kill_child;
+    (void) UX_close (STDOUT_FILENO);
   if (channel_err_type == process_channel_type_none)
-    if ((UX_close (STDERR_FILENO)) < 0) goto kill_child;
+    (void) UX_close (STDERR_FILENO);
   /* Assumption: STDIN_FILENO = 0, STDOUT_FILENO = 1, STDERR_FILENO = 2.  */
-  if ((UX_closefrom (3)) < 0) goto kill_child;
+  (void) UX_closefrom (3);
 
   /* Put the signal mask and handlers in a normal state.  */
   UX_initialize_child_signals ();

diff --git a/src/microcode/uxterm.c b/src/microcode/uxterm.c
index 58a50ed0e9a14eba45b858ade3f2249b4c25c024..8dcda50396fa782cd945be11ad829545b9057e93 100644 (file)
--- a/src/microcode/uxterm.c
+++ b/src/microcode/uxterm.c
@@ -759,7 +759,8 @@ open_pty_master_bsd (Tchannel * master_fd, const char ** master_fname)
          }
        if ((UX_access (slave_name, (R_OK | W_OK))) < 0)
          {
-           UX_close (fd);
+           /* FIXME: Need to handle EINTR.  */
+           (void) UX_close (fd);
            continue;
          }
        MAKE_CHANNEL (fd, channel_type_unix_pty_master, (*master_fd) =);