From a381a1b049f3a1b668e247419ea9c0226442e12d Mon Sep 17 00:00:00 2001
From: Martijn Dekker <martijn@inlv.org>
Date: Sat, 13 Nov 2021 00:49:01 +0100
Subject: [PATCH] Better fix for BUG_IFSISSET (re: 95294419)

With a better understanding of the code 1.5 years later, the
special-casing for IFS introduced in that commit seems like a hack.

The problem was not that the IFS node always exists but that it is
always considered to have a 'get' discipline function. Variables
with a 'get' discipline are considered set. This makes sense for
all variables except IFS.

The nv_isnull() macro is used to check if a variable is set. It
calls nv_hasget() to determine if the variable has a 'get'
discipline. So a better fix is for nv_hasget() always to return
false for IFS.

src/cmd/ksh93/bltins/test.c, src/cmd/ksh93/sh/macro.c:
- Remove special-casing for IFS.

src/cmd/ksh93/sh/nvdisc.c: nv_hasget():
- Always return false for IFS, taking local scope into account.
---
 src/cmd/ksh93/bltins/test.c | 2 --
 src/cmd/ksh93/sh/macro.c    | 7 +------
 src/cmd/ksh93/sh/nvdisc.c   | 6 ++++++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/cmd/ksh93/bltins/test.c b/src/cmd/ksh93/bltins/test.c
index e2e6dc641..7b1e83091 100644
--- a/src/cmd/ksh93/bltins/test.c
+++ b/src/cmd/ksh93/bltins/test.c
@@ -490,8 +490,6 @@ int test_unop(Shell_t *shp,register int op,register const char *arg)
 		}
 		if(ap = nv_arrayptr(np))
 			return(nv_arrayisset(np,ap));
-		if(*arg=='I' && strcmp(arg,"IFS")==0)
-			return(nv_getval(np)!=NULL);  /* avoid BUG_IFSISSET */
 		return(!nv_isnull(np));
 	    }
 	    default:
diff --git a/src/cmd/ksh93/sh/macro.c b/src/cmd/ksh93/sh/macro.c
index 6821d211b..cb493f67e 100644
--- a/src/cmd/ksh93/sh/macro.c
+++ b/src/cmd/ksh93/sh/macro.c
@@ -1318,13 +1318,8 @@ retry1:
 				flag &= ~NV_NOADD;
 			/*
 			 * Get a node pointer (np) to the parameter, if any.
-			 *
-			 * The IFS node always exists, so we must special-case IFS by checking if it has
-			 * a value; otherwise it always follows the code path for a set parameter, so is
-			 * not subject to 'set -u', and may test as set even after 'unset -v IFS'.
 			 */
-			if(!(*id=='I' && strcmp(id,"IFS")==0 && nv_getval(sh_scoped(mp->shp,IFSNOD)) == NIL(char*)))
-				np = nv_open(id,mp->shp->var_tree,flag|NV_NOFAIL);
+			np = nv_open(id,mp->shp->var_tree,flag|NV_NOFAIL);
 			if(!np)
 			{
 				sfprintf(mp->shp->strbuf,"%s%c",id,0);
diff --git a/src/cmd/ksh93/sh/nvdisc.c b/src/cmd/ksh93/sh/nvdisc.c
index c131c2a1a..14cbef77b 100644
--- a/src/cmd/ksh93/sh/nvdisc.c
+++ b/src/cmd/ksh93/sh/nvdisc.c
@@ -1497,9 +1497,15 @@ const Namdisc_t *nv_discfun(int which)
 	return(0);
 }
 
+/*
+ * Check if a variable node has a 'get' discipline.
+ * Used by the nv_isnull() macro (see include/name.h).
+ */
 int nv_hasget(Namval_t *np)
 {
 	register Namfun_t	*fp;
+	if(np==sh_scoped(&sh,IFSNOD))
+		return(0);	/* avoid BUG_IFSISSET: always return false for IFS */
 	for(fp=np->nvfun; fp; fp=fp->next)
 	{
 		if(!fp->disc || (!fp->disc->getnum && !fp->disc->getval))