[Partial patch] IFS and read builtin

[Harald van Dijk <harald@xxxxxxxxxxx>]

 Hi, as has been reported already dash currently has a bug where the 
read builtin ignores the read environment's IFS setting. As a result,

  echo a:b | { IFS=: read a b; echo $a; }

will write out a:b. I tried to see what changed between 0.5.5.1 and 
0.5.6, and found that the old code used bltinlookup("IFS"). So, I made 
the new code also use that. The above example works properly with the 
attached patch.

However, testing further, I found that there is bad interaction (even 
without my patch, where the IFS assignment below should just be ignored) 
between the code that handles variable assignments, and read. Try this:

  $ src/dash -c 'printf "a\t\tb\n" | { IFS=$(printf "\t") read a b c; 
echo "|$a|$b|$c|"; }'
  |a||b|
  $ src/dash -c 'printf "a\t\tb\n" | { IFS="$(printf "\t")" read a b c; 
echo "|$a|$b|$c|"; }'
  |a|b||

This happens because expbackq is correctly called without EXP_QUOTED, 
which makes it call recordregion, which isn't cleared by the time read 
calls ifsbreakup. I'm not sure how that should be solved, and if there 
are more cases where it would fail. The simplest way to solve this for 
read is to call removerecordregions(0) before recordregion(0, len - 1, 
0). I have tested that locally, and it works. I am not familiar enough 
with the code to judge whether the same situation can also happen in 
other cases that would also need fixing, which is why I have not 
included that part in the attached patch.

Cheers,
Harald

diff --git a/src/expand.c b/src/expand.c
index f2f964c..3ba1a38 100644
--- a/src/expand.c
+++ b/src/expand.c
@@ -205,7 +205,7 @@ expandarg(union node *arg, struct arglist *arglist, int flag)
 	 * TODO - EXP_REDIR
 	 */
 	if (flag & EXP_FULL) {
-		ifsbreakup(p, &exparg);
+		ifsbreakup(p, &exparg, 0);
 		*exparg.lastp = NULL;
 		exparg.lastp = &exparg.list;
 		expandmeta(exparg.list, flag);
@@ -1022,9 +1022,11 @@ recordregion(int start, int end, int nulonly)
  * Break the argument string into pieces based upon IFS and add the
  * strings to the argument list.  The regions of the string to be
  * searched for IFS characters have been stored by recordregion.
+ * If bltin is set, use bltinlookup to search for IFS in the
+ * environment of the currently executing built-in command.
  */
 void
-ifsbreakup(char *string, struct arglist *arglist)
+ifsbreakup(char *string, struct arglist *arglist, int bltin)
 {
 	struct ifsregion *ifsp;
 	struct strlist *sp;
@@ -1040,7 +1042,13 @@ ifsbreakup(char *string, struct arglist *arglist)
 	if (ifslastp != NULL) {
 		ifsspc = 0;
 		nulonly = 0;
-		realifs = ifsset() ? ifsval() : defifs;
+		if (!bltin)
+			realifs = ifsset() ? ifsval() : defifs;
+		else {
+			realifs = bltinlookup("IFS");
+			if (realifs == NULL)
+				realifs = defifs;
+		}
 		ifsp = &ifsfirst;
 		do {
 			p = string + ifsp->begoff;
diff --git a/src/expand.h b/src/expand.h
index 405af0b..8eb5f07 100644
--- a/src/expand.h
+++ b/src/expand.h
@@ -69,7 +69,7 @@ char *_rmescapes(char *, int);
 int casematch(union node *, char *);
 void recordregion(int, int, int);
 void removerecordregions(int); 
-void ifsbreakup(char *, struct arglist *);
+void ifsbreakup(char *, struct arglist *, int bltin);
 
 /* From arith.y */
 intmax_t arith(const char *);
diff --git a/src/miscbltin.c b/src/miscbltin.c
index 5ab1648..898ab09 100644
--- a/src/miscbltin.c
+++ b/src/miscbltin.c
@@ -87,7 +87,7 @@ readcmd_handle_line(char *line, char **ap, size_t len)
 	arglist.lastp = &arglist.list;
 	recordregion(0, len - 1, 0);
 	
-	ifsbreakup(s, &arglist);
+	ifsbreakup(s, &arglist, 1);
 	*arglist.lastp = NULL;
 	removerecordregions(0);