Module Name:    src
Committed By:   kre
Date:           Thu Oct  5 20:33:31 UTC 2023

Modified Files:
        src/bin/sh: miscbltin.c

Log Message:
If the read builtin is told to read into IFS, we must avoid doing
that until all current uses of IFS are complete (as we have IFS's
value cached in ifs - if IFS alters, ifs might point anywhere).
Handle this by deferring assignments to IFS until everything is done.
This makes us appear to comply with the (currently) proposed requirement
for read by POSIX that field splitting complete before vars are
assigned.   (Other shells, like dash, ksh93, yash, bosh behave like this)

That might end up being unspecified though, as other shells (bosh,
mksh) assign each field to its var as it is delimited (though bosh
appears to have bugs).   If we wanted to go that route, the issue here
could have been handled by re-doing the init of ifs after every
setvar() that is performed here (except the last, after which it is
no longer needed).

XXX pullup -10


To generate a diff of this commit:
cvs rdiff -u -r1.53 -r1.54 src/bin/sh/miscbltin.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/bin/sh/miscbltin.c
diff -u src/bin/sh/miscbltin.c:1.53 src/bin/sh/miscbltin.c:1.54
--- src/bin/sh/miscbltin.c:1.53	Sun Dec 11 08:23:10 2022
+++ src/bin/sh/miscbltin.c	Thu Oct  5 20:33:31 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: miscbltin.c,v 1.53 2022/12/11 08:23:10 kre Exp $	*/
+/*	$NetBSD: miscbltin.c,v 1.54 2023/10/05 20:33:31 kre Exp $	*/
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)miscbltin.c	8.4 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: miscbltin.c,v 1.53 2022/12/11 08:23:10 kre Exp $");
+__RCSID("$NetBSD: miscbltin.c,v 1.54 2023/10/05 20:33:31 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -102,6 +102,8 @@ readcmd(int argc, char **argv)
 	int is_ifs;
 	int saveall = 0;
 	ptrdiff_t wordlen = 0;
+	char *newifs = NULL;
+	struct stackmark mk;
 
 	end = '\n';				/* record delimiter */
 	rflag = 0;
@@ -132,6 +134,7 @@ readcmd(int argc, char **argv)
 	if ((ifs = bltinlookup("IFS", 1)) == NULL)
 		ifs = " \t\n";
 
+	setstackmark(&mk);
 	status = 0;
 	startword = 2;
 	STARTSTACKSTR(p);
@@ -198,8 +201,22 @@ readcmd(int argc, char **argv)
 			continue;
 		}
 
-		STACKSTRNUL(p);
-		setvar(*ap, stackblock(), 0);
+		if (equal(*ap, "IFS")) {
+			/*
+			 * we must not alter the value of IFS, as our
+			 * local "ifs" var is (perhaps) pointing at it,
+			 * at best we would be using data after free()
+			 * the next time we reference ifs - but that mem
+			 * may have been reused for something different.
+			 *
+			 * note that this might occur several times
+			 */
+			STPUTC('\0', p);
+			newifs = grabstackstr(p);
+		} else {
+			STACKSTRNUL(p);
+			setvar(*ap, stackblock(), 0);
+		}
 		ap++;
 		STARTSTACKSTR(p);
 		wordlen = 0;
@@ -217,11 +234,25 @@ readcmd(int argc, char **argv)
 			/* Don't remove non-whitespace unless it was naked */
 			break;
 	}
+
+	/*
+	 * If IFS was one of the variables named, we can finally set it now
+	 * (no further references to ifs will be made)
+	 */
+	if (newifs != NULL)
+		setvar("IFS", newifs, 0);
+
+	/*
+	 * Now we can assign to the final variable (which might
+	 * also be IFS, hence the ordering here)
+	 */
 	setvar(*ap, stackblock(), 0);
 
 	/* Set any remaining args to "" */
 	while (*++ap != NULL)
 		setvar(*ap, nullstr, 0);
+
+	popstackmark(&mk);
 	return status;
 }
 

Reply via email to