Re: A note for read builtin

[Dr. Werner Fink]

On Tue, Jun 22, 2010 at 04:50:54PM -0400, Chet Ramey wrote:
> On 6/18/10 10:05 AM, Dr. Werner Fink wrote:
> 
> > as now is visible the last command in the pipe sequence done
> > in the bash is a real sub process whereas in the ksh it is not.
> > 
> > The question rises: Why does the bash require a sub peocess/shell
> > for the final command of a pipe sequence.
> 
> It's an implementation decision: the code as it exists today is more
> straightforward.  Since Posix requires that all commands in a pipeline
> be executed in a subshell environment, though it allows the ksh behavior
> as an extension, there's been no requirement to change it.
> 
> If anyone would like to take a shot at changing the code to not fork when
> executing a command with a pipe for stdin, I'd be glad to take a look at
> the result.

OK ... after a deeper look into execute_cmd.c I'd like to suggest
something like in the attachment.  Anyone around with a better
solution? Maybe there is a way to emulate the first pipe commands
as a named fifo for the last element of the pipe which may avoid
moving stdin around.  Anyway the patch makes things like:

 bash/bash-4.1> ./bash -c 'echo a b | while read x y; do a=$x b=$y; done; echo 
$a $b'
 a b
 bash/bash-4.1> ./bash -c 'echo a b | read a b ; echo $a $b'
 a b

work.  The change of sh_coproc is simply to avoid a (correct)
compiler warning.


    Werner

-- 
  "Having a smoking section in a restaurant is like having
          a peeing section in a swimming pool." -- Edward Burr

--- execute_cmd.c
+++ execute_cmd.c	2010-06-23 09:58:23.119134909 +0200
@@ -1525,7 +1525,7 @@ static struct cpelement *cpl_search __P(
 static struct cpelement *cpl_searchbyname __P((char *));
 static void cpl_prune __P((void));
 
-Coproc sh_coproc = { 0, NO_PID, -1, -1, 0, 0 };
+Coproc sh_coproc = { 0, NO_PID, -1, -1, 0, 0, 0, 0 };
 
 cplist_t coproc_list = {0, 0, 0};
 
@@ -2053,7 +2053,7 @@ execute_pipeline (command, asynchronous,
      int asynchronous, pipe_in, pipe_out;
      struct fd_bitmap *fds_to_close;
 {
-  int prev, fildes[2], new_bitmap_size, dummyfd, ignore_return, exec_result;
+  int lstdin, prev, fildes[2], new_bitmap_size, dummyfd, ignore_return, exec_result;
   COMMAND *cmd;
   struct fd_bitmap *fd_bitmap;
 
@@ -2148,8 +2148,26 @@ execute_pipeline (command, asynchronous,
   /* Now execute the rightmost command in the pipeline.  */
   if (ignore_return && cmd)
     cmd->flags |= CMD_IGNORE_RETURN;
+
+  lstdin = -1;
+  if (!asynchronous && pipe_out == NO_PIPE && prev > 0)
+    {
+      lstdin = move_to_high_fd(0, 0, 64);
+      if (lstdin > 0)
+	{
+	  dup2(prev, 0);
+	  prev = NO_PIPE;
+	}
+    }
+
   exec_result = execute_command_internal (cmd, asynchronous, prev, pipe_out, fds_to_close);
 
+  if (lstdin > 0)
+    {
+      dup2(lstdin, 0);
+      close(lstdin);
+    }
+
   if (prev >= 0)
     close (prev);