changes to mapfile, read builtins for unbuffered reads

CWRU/CWRU.chlog

@@ -9723,7 +9723,7 @@ include/posixtime.h
 	- added some BSD convenience defines if they are not present
 
 parse.y,shell.h
-	- {save,restore}_parser_state: save amd restore shell_eof_token and
+	- {save,restore}_parser_state: save and restore shell_eof_token and
 	  pushed_string_list; change callers (e.g., xparse_dolparen) so they
 	  don't have to manage them
 
@@ -10129,3 +10129,25 @@ command.h
 subst.c	
 	- expand_subscript_string,expand_array_subscript: new functions to
 	  parse and expand-and-quote array subscripts. For future use
+
+				    5/3
+				    ---
+builtins/mapfile.def
+	- mapfile: if the delimiter is a newline, set unbuffered_read = 1
+	  for any file descriptor that isn't seekable and lseek sets errno
+	  to ESPIPE (pipes, FIFOs, maybe terminal devices). If it's not a
+	  newline, only allow buffered reads if the file descriptor is a
+	  regular file. Report from Koichi Murase <myoga.murase@gmail.com>
+
+builtins/read.def
+	- read_builtin: only set unbuffered_read = 1 if the input is coming
+	  from a pipe (which we can't seek on) or the input is a terminal and
+	  we want to read a specified number of characters or we're using a
+	  non-standard delimiter
+
+				    5/4
+				    ---
+
+builtins/mapfile.def
+	- mapfile: call zsyncfd before calling the callback. Suggested by
+	  Koichi Murase <myoga.murase@gmail.com>; we'll see how it goes

builtins/mapfile.def

@@ -2,7 +2,7 @@ This file is mapfile.def, from which is created mapfile.c.
 It implements the builtin "mapfile" in Bash.
 
 Copyright (C) 2005-2006 Rocky Bernstein for Free Software Foundation, Inc.
-Copyright (C) 2008-2020 Free Software Foundation, Inc.
+Copyright (C) 2008-2021 Free Software Foundation, Inc.
 
 This file is part of GNU Bash, the Bourne Again SHell.
 
@@ -68,6 +68,7 @@ $END
 #include <config.h>
 
 #include "builtins.h"
+#include "../bashtypes.h"
 #include "posixstat.h"
 
 #if defined (HAVE_UNISTD_H)
@@ -156,6 +157,7 @@ mapfile (fd, line_count_goal, origin, nskip, callback_quantum, callback, array_n
   size_t line_length;
   unsigned int array_index, line_count;
   SHELL_VAR *entry;
+  struct stat sb;
   int unbuffered_read;
   
   line = NULL;
@@ -180,19 +182,22 @@ mapfile (fd, line_count_goal, origin, nskip, callback_quantum, callback, array_n
     }
   else if (invisible_p (entry))
     VUNSETATTR (entry, att_invisible);	/* no longer invisible */
-      
+
   if (flags & MAPF_CLEARARRAY)
     array_flush (array_cell (entry));
 
 #ifndef __CYGWIN__
-  unbuffered_read = (lseek (fd, 0L, SEEK_CUR) < 0) && (errno == ESPIPE);
+  /* If the delimiter is a newline, turn on unbuffered reads for pipes
+     (terminals are ok). If the delimiter is not a newline, unbuffered reads
+     for every file descriptor that's not a regular file. */
+  if (delim == '\n')
+    unbuffered_read = (lseek (fd, 0L, SEEK_CUR) < 0) && (errno == ESPIPE);
+  else
+    unbuffered_read = (fstat (fd, &sb) != 0) || (S_ISREG (sb.st_mode) == 0);
 #else
   unbuffered_read = 1;
 #endif
 
-  if (delim != '\n')
-    unbuffered_read = 1;
-
   zreset ();
 
   /* Skip any lines at beginning of file? */
@@ -215,11 +220,11 @@ mapfile (fd, line_count_goal, origin, nskip, callback_quantum, callback, array_n
       /* Has a callback been registered and if so is it time to call it? */
       if (callback && line_count && (line_count % callback_quantum) == 0) 
 	{
-	  run_callback (callback, array_index, line);
-
 	  /* Reset the buffer for bash own stream. */
 	  if (unbuffered_read == 0)
 	    zsyncfd (fd);
+
+	  run_callback (callback, array_index, line);
 	}
 
       /* XXX - bad things can happen if the callback modifies ENTRY, e.g.,

builtins/read.def

@@ -593,11 +593,15 @@ read_builtin (list)
   add_unwind_protect (xfree, input_string);
 
   check_read_timeout ();
+  /* These only matter if edit == 0 */
   if ((nchars > 0) && (input_is_tty == 0) && ignore_delim)	/* read -N */
     unbuffered_read = 2;
+#if 0
   else if ((nchars > 0) || (delim != '\n') || input_is_pipe)
+#else
+  else if (((nchars > 0 || delim != '\n') && input_is_tty) || input_is_pipe)
     unbuffered_read = 1;
-
+#endif
   if (prompt && edit == 0)
     {
       fprintf (stderr, "%s", prompt);

subst.c

@@ -5582,7 +5582,11 @@ unlink_all_fifos ()
   for (i = 0; i < nfifo; i++)
     {
       fifo_list[i].proc = (pid_t)-1;
+#if defined (O_NONBLOCK)
       fd = open (fifo_list[i].file, O_RDWR|O_NONBLOCK);
+#else
+      fd = -1;
+#endif
       unlink_fifo (i);
       if (fd >= 0)
 	close (fd);

tests/RUN-ONE-TEST

@@ -1,4 +1,4 @@
-BUILD_DIR=/usr/local/build/chet/bash/bash-current
+BUILD_DIR=/usr/local/build/bash/bash-current
 THIS_SH=$BUILD_DIR/bash
 PATH=$PATH:$BUILD_DIR
 

tests/read.right

@@ -30,6 +30,7 @@ argv[1] = <foo>
 argv[1] = <foo>
 argv[1] = < foo>
 a = abcdefg
+xyz
 a = xyz
 a = -xyz 123-
 a = abc
@@ -46,6 +47,7 @@ abcde
 abcde
 ./read3.sub: line 17: read: -1: invalid number
 abc
+defg
 ab
 abc
 #
@@ -76,3 +78,7 @@ unset or null 3
 [?2004h[?2004l
 timeout 3: ok
 unset or null 3
+timeout 4: ok
+abcde
+abcde
+one

tests/read.tests

@@ -112,3 +112,6 @@ ${THIS_SH} ./read6.sub
 
 # test behavior of readline timeouts
 ${THIS_SH} ./read7.sub
+
+# test behavior of read -n and read -d on regular files
+${THIS_SH} ./read8.sub

tests/read1.sub

@@ -15,6 +15,7 @@ a=7
 echo 'abcdefg|xyz' | {
 	read -d '|' a 
 	echo a = "${a-unset}"
+	cat -	# make sure we don't read too much
 }
 
 echo xyz 123 | {

tests/read3.sub

@@ -20,6 +20,7 @@ read -n -1
 echo abcdefg | {
 	read -n 3 xyz
 	echo $xyz
+	cat -	# make sure we don't read too much
 }
 
 # fewer chars than specified

tests/read8.sub

@@ -0,0 +1,15 @@
+tmpf=$TMPDIR/tmp-$$
+printf "%s\n" "one two three four" > $tmpf
+
+# make sure we rewind the input properly when reading a specific number of
+# characters or using a non-standard delimiter from a regular file
+
+exec <$tmpf
+read -n 4 input && echo "$input"
+cat -
+
+exec <$tmpf
+read -d ' ' input && echo "$input"
+cat -
+
+rm -f $tmpf