From 5aba0c725154425352ea61a53639562fbfe4498a Mon Sep 17 00:00:00 2001
From: hyenias <58673227+hyenias@users.noreply.github.com>
Date: Sun, 7 Mar 2021 23:19:36 -0500
Subject: [PATCH] Fix set/unset state for short integer (typeset -si) (#211)

This commit fixes at least three bugs:
1. When issuing 'typeset -p' for unset variables typeset as short
   integer, a value of 0 was incorrectly diplayed.
2. ${x=y} and ${x:=y} were still broken for short integer types
   (re: 9f2389ed). ${x+set} and ${x:+nonempty} were also broken.
3. A memory fault could occur if typeset -l followed a -s option
   with integers. Additonally, now the last -s/-l wins out as the
   option to utilize instead of it always being short.

src/cmd/ksh93/include/name.h:
- Fix the nv_isnull() macro by removing the direct exclusion of
  short integers from this set/unset test. This breaks few things
  (only ${.sh.subshell} and ${.sh.level}, as far as we can tell)
  while potentially correcting many aspects of short integer use
  (at least bugs 1 and 2 above), as this macro is widely used.
- union Value: add new pid_t *pidp pointer member for PID values
  (see further below).

src/cmd/ksh93/bltins/typeset.c: b_typeset():
- To fix bug 3 above, unset the 'shortint' flag and NV_SHORT
  attribute bit upon encountering the -l optiobn.

*** To fix ${.sh.subshell} to work with the new nv_isnull():

src/cmd/ksh93/sh/defs.h:
- Add new 'realsubshell' member to the shgd (aka shp->gd) struct
  which will be the integer value for ${.sh.subshell}.

src/cmd/ksh93/sh/init.c,
src/cmd/ksh93/data/variables.c:
- Initialize SH_SUBSHELLNOD as a pointer to shgd->realsubshell
  instead of using a short value (.s) directly. Using a pointer
  allows nv_isnull() to return a positive for ${.sh.subshell} as
  a non-null pointer is what it checks for.
- While we're at it, initialize PPIDNOD ($PPID) and SH_PIDNOD
  (${.sh.pid}) using the new pdip union member, which is more
  correct as they are values of type pid_t.

src/cmd/ksh93/sh/subshell.c,
src/cmd/ksh93/sh/xec.c:
- Update the ${.sh.subshell} increases/decreases to refer to
  shgd->realsubshell (a.k.a. shp->gd->realsubshell).

*** To fix ${.sh.level} after changing nv_isnull():

src/cmd/ksh93/sh/macro.c: varsub():
- Add a specific exception for SH_LEVLNOD to the nv_isnull() test,
  so that ${.sh.level} is always considered to be set. Its handling
  throughout the code is too complex/special for a simple fix, so
  we have to special-case it, at least for now.

*** Regression test additions:

src/cmd/ksh93/tests/attributes.sh:
- Add in missing short integer tests and correct the one that
  existed. The -si test now yields 'typeset -x -r -s -i foo'
  instead of 'typeset -x -r -s -i foo=0' which brings it in line
  with all the others.
- Add in some other -l attribute tests for floats. Note, -lX test
  was not added as the size of long double is platform dependent.

src/cmd/ksh93/tests/variables.sh:
- Add tests for ${x=y} and ${x:=y} used on short int variables.

Co-authored-by: Martijn Dekker <martijn@inlv.org>
---
 NEWS                              |  3 +++
 src/cmd/ksh93/bltins/typeset.c    |  5 +++++
 src/cmd/ksh93/data/variables.c    |  2 +-
 src/cmd/ksh93/include/defs.h      |  1 +
 src/cmd/ksh93/include/name.h      |  3 ++-
 src/cmd/ksh93/sh/init.c           |  5 +++--
 src/cmd/ksh93/sh/macro.c          |  2 +-
 src/cmd/ksh93/sh/subshell.c       |  6 +++---
 src/cmd/ksh93/sh/xec.c            |  2 +-
 src/cmd/ksh93/tests/attributes.sh | 10 +++++++++-
 src/cmd/ksh93/tests/variables.sh  | 15 +++++++++++++++
 11 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 5fccf2eee..a00fcfe1e 100644
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,9 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 
 2021-03-07:
 
+- Fixed the typeset -p display of short integers without an assigned value.
+  Also, the last -s or -l attribute option supplied for an integer is used.
+
 - Fixed a bug with -G/--globstar introduced on 2020-08-09: patterns did not
   match anything if any pathname component was '.' or '..', e.g. '**/./glob.c'
   never matched. The 2020-08-09 fix does still apply to patterns like '.*'.
diff --git a/src/cmd/ksh93/bltins/typeset.c b/src/cmd/ksh93/bltins/typeset.c
index 316a6bb3c..7a1ce24e7 100644
--- a/src/cmd/ksh93/bltins/typeset.c
+++ b/src/cmd/ksh93/bltins/typeset.c
@@ -328,6 +328,11 @@ int    b_typeset(int argc,register char *argv[],Shbltin_t *context)
 					flag |= NV_INTEGER;
 				break;
 			case 'l':
+				if(shortint)
+				{
+					shortint = 0;
+					flag &= ~NV_SHORT;
+				}
 				tdata.wctname = e_tolower;
 				flag |= NV_UTOL;
 				break;
diff --git a/src/cmd/ksh93/data/variables.c b/src/cmd/ksh93/data/variables.c
index 76999d0a1..a029e3bd5 100644
--- a/src/cmd/ksh93/data/variables.c
+++ b/src/cmd/ksh93/data/variables.c
@@ -95,7 +95,7 @@ const struct shtable2 shtab_variables[] =
 	".sh.command",	0,				(char*)0,
 	".sh.file",	0,				(char*)0,
 	".sh.fun",	0,				(char*)0,
-	".sh.subshell",	NV_INTEGER|NV_SHORT|NV_NOFREE,	(char*)0,
+	".sh.subshell",	NV_INTEGER|NV_NOFREE,		(char*)0,
 	".sh.level",	0,				(char*)0,
 	".sh.lineno",	NV_INTEGER|NV_NOFREE,		(char*)0,
 	".sh.stats",	0,				(char*)0,
diff --git a/src/cmd/ksh93/include/defs.h b/src/cmd/ksh93/include/defs.h
index daac1db36..41b1729d1 100644
--- a/src/cmd/ksh93/include/defs.h
+++ b/src/cmd/ksh93/include/defs.h
@@ -138,6 +138,7 @@ struct shared
 	pid_t		pid;		/* $$, the main shell's PID (invariable) */
 	pid_t		ppid;		/* $PPID, the main shell's parent's PID */
 	pid_t		current_pid;	/* ${.sh.pid}, PID of current ksh process (updates when subshell forks) */
+	int		realsubshell;	/* ${.sh.subshell}, actual subshell level (including virtual and forked) */
 	unsigned char	sigruntime[2];
 	Namval_t	*bltin_nodes;
 	Namval_t	*bltin_cmds;
diff --git a/src/cmd/ksh93/include/name.h b/src/cmd/ksh93/include/name.h
index bd727dfb0..cba492681 100644
--- a/src/cmd/ksh93/include/name.h
+++ b/src/cmd/ksh93/include/name.h
@@ -42,6 +42,7 @@ union Value
 	int			i;
 	unsigned int		u;
 	int32_t			*lp;
+	pid_t			*pidp;
 	Sflong_t		*llp;	/* for long long arithmetic */
 	int16_t			s;
 	int16_t			*sp;
@@ -171,7 +172,7 @@ struct Ufunction
 #undef nv_size
 #define nv_size(np)	((np)->nvsize)
 #define _nv_hasget(np)  ((np)->nvfun && (np)->nvfun->disc && nv_hasget(np))
-#define nv_isnull(np)	(!(np)->nvalue.cp && (nv_isattr(np,NV_SHORT|NV_INTEGER)!=(NV_SHORT|NV_INTEGER)) && !_nv_hasget(np))
+#define nv_isnull(np)	(!(np)->nvalue.cp && !_nv_hasget(np))
 
 /* ...	for arrays */
 
diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c
index caf16abd5..bc226e9b4 100644
--- a/src/cmd/ksh93/sh/init.c
+++ b/src/cmd/ksh93/sh/init.c
@@ -1855,8 +1855,9 @@ static Init_t *nv_init(Shell_t *shp)
 	nv_stack(LCNUMNOD, &ip->LC_NUM_init);
 	nv_stack(LANGNOD, &ip->LANG_init);
 #endif /* _hdr_locale */
-	(PPIDNOD)->nvalue.lp = (&shp->gd->ppid);
-	(SH_PIDNOD)->nvalue.lp = (&shp->gd->current_pid);
+	(PPIDNOD)->nvalue.pidp = (&shp->gd->ppid);
+	(SH_PIDNOD)->nvalue.pidp = (&shp->gd->current_pid);
+	(SH_SUBSHELLNOD)->nvalue.ip = (&shp->gd->realsubshell);
 	(TMOUTNOD)->nvalue.lp = (&shp->st.tmout);
 	(MCHKNOD)->nvalue.lp = (&sh_mailchk);
 	(OPTINDNOD)->nvalue.lp = (&shp->st.optindex);
diff --git a/src/cmd/ksh93/sh/macro.c b/src/cmd/ksh93/sh/macro.c
index 3f7f69496..7c113dd99 100644
--- a/src/cmd/ksh93/sh/macro.c
+++ b/src/cmd/ksh93/sh/macro.c
@@ -1409,7 +1409,7 @@ retry1:
 		if(np && type==M_BRACE && mp->shp->argaddr)
 			nv_optimize(np);  /* needed before calling nv_isnull() */
 #endif /* SHOPT_OPTIMIZE */
-		if(np && (type==M_BRACE ? !nv_isnull(np) : (type==M_TREE || !c || !ap)))
+		if(np && (type==M_BRACE ? (!nv_isnull(np) || np==SH_LEVELNOD) : (type==M_TREE || !c || !ap)))
 		{
 			/* Either the parameter is set, or it's a special type of expansion where 'unset' doesn't apply. */
 			char *savptr;
diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c
index e2b89393a..28d1c40d1 100644
--- a/src/cmd/ksh93/sh/subshell.c
+++ b/src/cmd/ksh93/sh/subshell.c
@@ -216,7 +216,7 @@ void sh_subfork(void)
 		shp->st.trapcom[0] = (comsub==2 ? NULL : trap);
 		shp->savesig = 0;
 		/* sh_fork() increases ${.sh.subshell} but we forked an existing virtual subshell, so undo */
-		SH_SUBSHELLNOD->nvalue.s--;
+		shgd->realsubshell--;
 	}
 }
 
@@ -537,7 +537,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	savst = shp->st;
 	sh_pushcontext(shp,&buff,SH_JMPSUB);
 	shp->subshell++;		/* increase level of virtual subshells */
-	SH_SUBSHELLNOD->nvalue.s++;	/* increase ${.sh.subshell} */
+	shgd->realsubshell++;		/* increase ${.sh.subshell} */
 	sp->prev = subshell_data;
 	sp->shp = shp;
 	sp->sig = 0;
@@ -883,7 +883,7 @@ Sfio_t *sh_subshell(Shell_t *shp,Shnode_t *t, volatile int flags, int comsub)
 	if(shp->subshell)
 	{
 		shp->subshell--;		/* decrease level of virtual subshells */
-		SH_SUBSHELLNOD->nvalue.s--;	/* decrease ${.sh.subshell} */
+		shgd->realsubshell--;		/* decrease ${.sh.subshell} */
 	}
 	subshell_data = sp->prev;
 	if(!argsav  ||  argsav->dolrefcnt==argcnt)
diff --git a/src/cmd/ksh93/sh/xec.c b/src/cmd/ksh93/sh/xec.c
index d8603bd67..e4caa2daa 100644
--- a/src/cmd/ksh93/sh/xec.c
+++ b/src/cmd/ksh93/sh/xec.c
@@ -2949,7 +2949,7 @@ pid_t _sh_fork(Shell_t *shp,register pid_t parent,int flags,int *jobid)
 	/* except for those `lost' by trap   */
 	if(!(flags&FSHOWME))
 		sh_sigreset(2);
-	SH_SUBSHELLNOD->nvalue.s++;	/* increase ${.sh.subshell} */
+	shgd->realsubshell++;		/* increase ${.sh.subshell} */
 	shp->subshell = 0;		/* zero virtual subshells */
 	shp->comsub = 0;
 	shp->spid = 0;
diff --git a/src/cmd/ksh93/tests/attributes.sh b/src/cmd/ksh93/tests/attributes.sh
index ddcdc206c..4d0c07277 100755
--- a/src/cmd/ksh93/tests/attributes.sh
+++ b/src/cmd/ksh93/tests/attributes.sh
@@ -513,11 +513,18 @@ typeset -A expect=(
 	[b]='typeset -x -r -b foo'
 	[i]='typeset -x -r -i foo'
 	[i37]='typeset -x -r -i 37 foo'
+	[ils]='typeset -x -r -s -i foo'
+	[isl]='typeset -x -r -l -i foo'
 	[l]='typeset -x -r -l foo'
+	[lF]='typeset -x -r -l -F foo'
+	[lE]='typeset -x -r -l -E foo'
 	[n]='typeset -n -r foo'
 	[s]='typeset -x -r foo'
-	[si]='typeset -x -r -s -i foo=0'
+	[si]='typeset -x -r -s -i foo'
 	[u]='typeset -x -r -u foo'
+	[ui]='typeset -x -r -u -i foo'
+	[usi]='typeset -x -r -s -u -i foo'
+	[uli]='typeset -x -r -l -u -i foo'
 	[A]='typeset -x -r -A foo=()'
 	[C]='typeset -x -r foo=()'
 	[E]='typeset -x -r -E foo'
@@ -532,6 +539,7 @@ typeset -A expect=(
 	[R]='typeset -x -r -R 0 foo'
 	[R17]='typeset -x -r -R 17 foo'
 	[X17]='typeset -x -r -X 17 foo'
+	[lX17]='typeset -x -r -l -X 17 foo'
 	[S]='typeset -x -r foo'
 	[T]='typeset -x -r foo'
 	[Z]='typeset -x -r -Z 0 -R 0 foo'
diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh
index 2a4565061..724d3b6e8 100755
--- a/src/cmd/ksh93/tests/variables.sh
+++ b/src/cmd/ksh93/tests/variables.sh
@@ -1201,6 +1201,7 @@ got=$(some_func() { :; }; trap some_func DEBUG; trap - DEBUG; print -r "${.sh.fu
 # =====
 # Before 2021-03-06, ${foo=bar} and ${foo:=bar} did not work if `foo` had a numeric type
 # https://github.com/ksh93/ksh/issues/157
+# https://github.com/ksh93/ksh/pull/211#issuecomment-792336825
 
 unset a b
 typeset -i a
@@ -1209,6 +1210,13 @@ got=${a=b}
 [[ $got == 42 ]] || err_exit "\${a=b}: expansion not working for integer type (expected '42', got '$got')"
 [[ $a == 42 ]] || err_exit "\${a=b}: a was not assigned the correct integer value (expected '42', got '$a')"
 
+unset a b
+typeset -si a
+b=3+39
+got=${a=b}
+[[ $got == 42 ]] || err_exit "\${a=b}: expansion not working for short integer type (expected '42', got '$got')"
+[[ $a == 42 ]] || err_exit "\${a=b}: a was not assigned the correct short integer value (expected '42', got '$a')"
+
 unset a b
 typeset -F a
 b=3.75+38.25
@@ -1224,6 +1232,13 @@ got=${a:=b}
 [[ $got == 42 ]] || err_exit "\${a:=b}: expansion not working for integer type (expected '42', got '$got')"
 [[ $a == 42 ]] || err_exit "\${a:=b}: a was not assigned the correct integer value (expected '42', got '$a')"
 
+unset a b
+typeset -si a
+b=3+39
+got=${a:=b}
+[[ $got == 42 ]] || err_exit "\${a:=b}: expansion not working for short integer type (expected '42', got '$got')"
+[[ $a == 42 ]] || err_exit "\${a:=b}: a was not assigned the correct short integer value (expected '42', got '$a')"
+
 unset a b
 typeset -F a
 b=3.75+38.25