From: Taylor R Campbell <campbell@mumble.net>
Date: Fri, 10 Jun 2011 00:33:50 +0000 (+0000)
Subject: Fix use of the close system call.
X-Git-Tag: release-9.1.0~24
X-Git-Url: https://birchwood-abbey.net/git?a=commitdiff_plain;h=3683373013dca9fe379d4361be85146d88c61e14;p=mit-scheme.git

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 189830009..497d13374 100644
--- 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 a9b2504e6..02679a69e 100644
--- 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 46802430a..18cdbec53 100644
--- 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 df221168f..353620709 100644
--- 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 ffe7f7d1e..c52441c41 100644
--- 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 58a50ed0e..8dcda5039 100644
--- 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) =);