subshell output swallowed (patch)

[Egmont Koblinger <egmont uhulinux hu>]

Hi,

When an external command is executed in mc with subshell support, quite
often some characters of its output are swallowed. Maybe it's most noticable
with the "ls" command in a directory where there are a lot of files and "ls"
outputs them in more columns. Just launch a terminal emulator, start mc,
press ^O so that the panels disappear, and run "ls" a several times. Sure
you'll see some occasions when the columns are not lined up correctly.

For example, in a 70-column wide terminal, standing in the source tree of
Python-2.4.2 in the Lib directory, an ls now produced this output to me:

[...]
fileinput.py        plat-irix6            toaiff.py
fnmatch.py          plat-linux2           tokenize.py
formatter.py        plat-mac              token.py
fpformat.py    plat-netbsd1          traceback.py
ftplib.py           plat-next3            trace.py
__future__.py       plat-os2emx           tzparse.py
getopt.py           plat-riscos           types.py
[...]

The bug is reproducible in different terminal emulators, and does not occur
if "ls" is executed outside mc, or inside "mc -u".

An "strace -e trace=read,write" of a similar case showed this:

[...]
read(4, "\33[0mcopy.py\33[0m            \33[0mi"..., 100) = 100 
write(1, "\33[0mcopy.py\33[0m            \33[0mi"..., 100) = 100
read(4, "m         \33[0m_strptime.py\33[0m  "..., 100) = 62
write(1, "m         \33[0m_strptime.py\33[0m  "..., 62) = 62
read(4, "\33[0mcopy_reg.py\33[0m        \33[0mi"..., 100) = 100
write(1, "\33[0mcopy_reg.py\33[0m        \33[0mi"..., 100) = 53
--- SIGCHLD (Child exited) @ 0 (0) ---
read(4, "py\33[0m        \33[0msubprocess.py\33"..., 100) = 100
write(1, "py\33[0m        \33[0msubprocess.py\33"..., 100) = 100
read(4, "mghdr.py\33[0m           \33[0mpipes"..., 100) = 100   
write(1, "mghdr.py\33[0m           \33[0mpipes"..., 100) = 100  
[...]

So, when "ls" exits, mc receives a sigchild, which interrupts the write call
that transfers the output of ls from the subshell to its real output, but
the interrupted call is not finished, in this particular case 47 bytes are
skipped.

See the attached patch which fixes this problem.



bye,

Egmont

diff -Naur mc-4.6.1.orig/ChangeLog mc-4.6.1/ChangeLog
--- mc-4.6.1.orig/ChangeLog	2005-07-23 18:51:11.000000000 +0200
+++ mc-4.6.1/ChangeLog	2006-01-02 19:00:41.000000000 +0100
@@ -1,3 +1,8 @@
+2006-01-02  Egmont Koblinger  <egmont uhulinux hu>
+
+	* src/subshell.c: restart write() calls interrupted by sigchld
+	which lead to some bytes being swallowed from the subshell's output.
+
 2005-07-23  Roland Illig  <roland illig gmx de>
 
 	* configure.ac: getgrouplist() is not used anymore, so there' no
diff -Naur mc-4.6.1.orig/src/subshell.c mc-4.6.1/src/subshell.c
--- mc-4.6.1.orig/src/subshell.c	2005-06-07 11:19:19.000000000 +0200
+++ mc-4.6.1/src/subshell.c	2006-01-02 18:58:01.000000000 +0100
@@ -150,6 +150,30 @@
 
 
 /*
+ *  Write all data, even if the write() call is interrupted.
+ */
+static ssize_t
+write_all (int fd, const void *buf, size_t count)
+{
+    ssize_t ret;
+    ssize_t written = 0;
+    while (count > 0) {
+	ret = write (fd, buf, count);
+	if (ret < 0) {
+	    if (errno == EINTR) {
+		continue;
+	    } else {
+		return written > 0 ? written : ret;
+	    }
+	}
+	buf += ret;
+	count -= ret;
+	written += ret;
+    }
+    return written;
+}
+
+/*
  *  Prepare child process to running the shell and run it.
  *
  *  Modifies the global variables (in the child process only):
@@ -476,7 +500,7 @@
 		    tcsh_fifo);
 	break;
     }
-    write (subshell_pty, precmd, strlen (precmd));
+    write_all (subshell_pty, precmd, strlen (precmd));
 
     /* Wait until the subshell has started up and processed the command */
 
@@ -533,16 +557,16 @@
 	    subshell_state = ACTIVE;
 	    /* FIXME: possibly take out this hack; the user can
 	       re-play it by hitting C-hyphen a few times! */
-	    write (subshell_pty, " \b", 2);  /* Hack to make prompt reappear */
+	    write_all (subshell_pty, " \b", 2);  /* Hack to make prompt reappear */
 	}
     }
     else  /* MC has passed us a user command */
     {
 	if (how == QUIETLY)
-	    write (subshell_pty, " ", 1);
+	    write_all (subshell_pty, " ", 1);
 	/* FIXME: if command is long (>8KB ?) we go comma */
-	write (subshell_pty, command, strlen (command));
-	write (subshell_pty, "\n", 1);
+	write_all (subshell_pty, command, strlen (command));
+	write_all (subshell_pty, "\n", 1);
 	subshell_state = RUNNING_COMMAND;
 	subshell_ready = FALSE;
     }
@@ -767,21 +791,21 @@
 
     /* The initial space keeps this out of the command history (in bash
        because we set "HISTCONTROL=ignorespace") */
-    write (subshell_pty, " cd ", 4);
+    write_all (subshell_pty, " cd ", 4);
     if (*directory) {
 	char *temp = subshell_name_quote (directory);
 	if (temp) {
-	    write (subshell_pty, temp, strlen (temp));
+	    write_all (subshell_pty, temp, strlen (temp));
 	    g_free (temp);
 	} else {
 	    /* Should not happen unless the directory name is so long
 	       that we don't have memory to quote it.  */
-	    write (subshell_pty, ".", 1);
+	    write_all (subshell_pty, ".", 1);
 	}
     } else {
-	write (subshell_pty, "/", 1);
+	write_all (subshell_pty, "/", 1);
     }
-    write (subshell_pty, "\n", 1);
+    write_all (subshell_pty, "\n", 1);
 
     subshell_state = RUNNING_COMMAND;
     feed_subshell (QUIETLY, FALSE);
@@ -952,7 +976,7 @@
 	    }
 
 	    if (how == VISIBLY)
-		write (STDOUT_FILENO, pty_buffer, bytes);
+		write_all (STDOUT_FILENO, pty_buffer, bytes);
 	}
 
 	else if (FD_ISSET (subshell_pipe[READ], &read_set))
@@ -994,13 +1018,13 @@
 
 	    for (i = 0; i < bytes; ++i)
 		if (pty_buffer[i] == subshell_switch_key) {
-		    write (subshell_pty, pty_buffer, i);
+		    write_all (subshell_pty, pty_buffer, i);
 		    if (subshell_ready)
 			subshell_state = INACTIVE;
 		    return TRUE;
 		}
 
-	    write (subshell_pty, pty_buffer, bytes);
+	    write_all (subshell_pty, pty_buffer, bytes);
 	    subshell_ready = FALSE;
 	} else {
 	    return FALSE;