nvalue sanitisation part 3: union Value (re: 0686c70, 0ebaab3)

The two referenced commits have eliminatd almost all use of
non-pointer members of union Value. This commit eliminates the two
last ones: uses of nvalue.i in SHOPT_KIA code in parse.c. The union
now consists entirely of pointers, eliminating one significant risk
of undefined behaviour.

With that, we can eliminate nvalue.s and nvalue.i, and their
special handling in various bits of the code, mainly nv_getval()
and nv_putval() in name.c, and the nv_isnonptr() macro in name.h.

The nval.h attribute bits macros NV_INT16P and NV_UINT16P (short
int with pointer value) are removed, as they are now redundant.

src/cmd/ksh93/bltins/enum.c

@@ -183,7 +183,7 @@ static void put_enum(Namval_t* np,const char *val,int flags,Namfun_t *fp)
 			n = strcmp(v,val);
 		if(n==0)
 		{
-			nv_putv(np, (char*)&i, NV_UINT16P, fp);
+			nv_putv(np, (char*)&i, NV_UINT16, fp);
 			return;
 		}
 		i++;
@@ -260,7 +260,7 @@ int b_enum(int argc, char** argv, Shbltin_t *context)
 		tp = nv_open(sfstruse(sh.strbuf), sh.var_tree, NV_VARNAME);
 		n = sz;
 		i = 0;
-		nv_onattr(tp, NV_UINT16P);
+		nv_onattr(tp, NV_UINT16);
 		nv_putval(tp, (char*)&i, NV_INTEGER);
 		nv_putsub(np, NULL, ARRAY_SCAN);
 		do

src/cmd/ksh93/bltins/typeset.c

@@ -296,7 +296,7 @@ int    b_typeset(int argc,char *argv[],Shbltin_t *context)
 				if(shortint)
 				{
 					shortint = 0;
-					flag &= ~NV_INT16P;
+					flag &= ~NV_INT16;
 				}
 				if(n=='E')
 				{
@@ -375,7 +375,7 @@ int    b_typeset(int argc,char *argv[],Shbltin_t *context)
 				if(shortint)
 				{
 					flag &= ~NV_LONG;
-					flag |= NV_INT16P;
+					flag |= NV_INT16;
 				}
 				else
 					flag |= NV_INTEGER;
@@ -384,8 +384,8 @@ int    b_typeset(int argc,char *argv[],Shbltin_t *context)
 				if(shortint)
 				{
 					shortint = 0;
-					/* Turn off the NV_INT16P bits except the NV_INTEGER bit */
-					flag &= ~(NV_INT16P & ~NV_INTEGER);
+					/* Turn off the NV_INT16 bits except the NV_INTEGER bit */
+					flag &= ~(NV_INT16 & ~NV_INTEGER);
 				}
 				tdata.wctname = e_tolower;
 				flag |= NV_UTOL;
@@ -411,7 +411,7 @@ int    b_typeset(int argc,char *argv[],Shbltin_t *context)
 					if(flag&NV_INTEGER)
 					{
 						flag &= ~NV_LONG;
-						flag |= NV_INT16P;
+						flag |= NV_INT16;
 					}
 				}
 				break;

src/cmd/ksh93/data/variables.c

@@ -94,7 +94,7 @@ const struct shtable2 shtab_variables[] =
 	".sh.file",	0,				NULL,
 	".sh.fun",	0,				NULL,
 	".sh.subshell",	NV_INTEGER|NV_NOFREE,		NULL,
-	".sh.level",	NV_INT16P|NV_NOFREE|NV_RDONLY,	NULL,
+	".sh.level",	NV_INT16|NV_NOFREE|NV_RDONLY,	NULL,
 	".sh.lineno",	NV_INTEGER|NV_NOFREE,		NULL,
 	".sh.stats",	0,				NULL,
 	".sh.math",	0,				NULL,

src/cmd/ksh93/include/name.h

@@ -34,11 +34,9 @@ union Value
 {
 	const char		*cp;
 	int			*ip;
-	int			i;
 	int32_t			*lp;
 	pid_t			*pidp;
 	Sflong_t		*llp;	/* for long long arithmetic */
-	int16_t			s;
 	int16_t			*sp;
 	double			*dp;	/* for floating point arithmetic */
 	Sfdouble_t		*ldp;	/* for long floating point arithmetic */
@@ -160,9 +158,7 @@ struct Ufunction
 #undef nv_size
 #define nv_size(np)	((np)->nvsize)
 #define _nv_hasget(np)  ((np)->nvfun && (np)->nvfun->disc && nv_hasget(np))
-/* for nv_isnull we must exclude non-pointer value attributes (NV_INT16, NV_UINT16) before accessing cp in union Value */
-#define nv_isnonptr(np)	(nv_isattr(np,NV_INT16P)==NV_INT16)
-#define nv_isnull(np)	(!nv_isnonptr(np) && !(np)->nvalue.cp && !_nv_hasget(np))
+#define nv_isnull(np)	(!(np)->nvalue.cp && !_nv_hasget(np))
 
 /* ...	for arrays */
 

src/cmd/ksh93/include/nval.h

@@ -194,10 +194,7 @@ struct Namval
 #define NV_PUBLIC	(~(NV_NOSCOPE|NV_ASSIGN|NV_IDENT|NV_VARNAME|NV_NOADD))
 
 /* numeric types */
-/* NV_INT16 and NV_UINT16 store values directly in the node; all the others use pointers */
-#define NV_INT16P	(NV_LJUST|NV_SHORT|NV_INTEGER)
 #define NV_INT16	(NV_SHORT|NV_INTEGER)
-#define NV_UINT16P	(NV_LJUST|NV_UNSIGN|NV_SHORT|NV_INTEGER)
 #define NV_UINT16	(NV_UNSIGN|NV_SHORT|NV_INTEGER)
 #define NV_INT32	(NV_INTEGER)
 #define NV_UINT32	(NV_UNSIGN|NV_INTEGER)

src/cmd/ksh93/sh/array.c

@@ -563,10 +563,6 @@ static char *array_getval(Namval_t *np, Namfun_t *disc)
 		}
 		return cp;
 	}
-#if SHOPT_FIXEDARRAY
-	if(ap->fixed && nv_isattr(np,NV_INT16P|NV_DOUBLE) == NV_INT16)
-		np->nvalue.s = *np->nvalue.sp;
-#endif /* SHOPT_FIXEDARRAY */
 	return nv_getv(np,&ap->hdr);
 }
 
@@ -1757,11 +1753,11 @@ void *nv_associative(Namval_t *np,const char *sp,int mode)
 				if(sh.subshell)
 					sh_assignok(np,1);
 				/*
-				 * For enum types (NV_UINT16P with discipline ENUM_disc), nelem should not
+				 * For enum types (NV_UINT16 with discipline ENUM_disc), nelem should not
 				 * increase or 'unset' will fail to completely unset such an array.
 				 */
 				if((!ap->header.scope || !nv_search(sp,dtvnext(ap->header.table),0))
-				&& !(type==NV_UINT16P && nv_hasdisc(np, &ENUM_disc)))
+				&& !(type==NV_UINT16 && nv_hasdisc(np, &ENUM_disc)))
 					ap->header.nelem++;
 				if(nv_isnull(mp))
 				{

src/cmd/ksh93/sh/init.c

@@ -1965,8 +1965,7 @@ Dt_t *sh_inittree(const struct shtable2 *name_vals)
 		{
 			if(name_vals == shtab_variables)
 				np->nvfun = &sh.nvfun;
-			if(!nv_isnonptr(np))
-				np->nvalue.cp = (char*)tp->sh_value;
+			np->nvalue.cp = (char*)tp->sh_value;
 		}
 		nv_setattr(np,tp->sh_number);
 		if(nv_isattr(np,NV_TABLE))

src/cmd/ksh93/sh/name.c

@@ -1606,7 +1606,6 @@ void nv_putval(Namval_t *np, const char *string, int flags)
 	union Value	*up;
 	unsigned int	size = 0;
 	int		was_local = nv_local;
-	union Value	u;
 #if SHOPT_FIXEDARRAY
 	Namarr_t	*ap;
 #endif /* SHOPT_FIXEDARRAY */
@@ -1660,18 +1659,10 @@ void nv_putval(Namval_t *np, const char *string, int flags)
 		return;
 	}
 	up= &np->nvalue;
-	if(nv_isattr(np,NV_INT16P|NV_DOUBLE) == NV_INT16)
-	{
-		if(!np->nvalue.up || !nv_isarray(np))
-		{
-			up = &u;
-			up->up = &np->nvalue;
-		}
-	}
 #if SHOPT_FIXEDARRAY
-	else if(np->nvalue.up && nv_isarray(np) && (ap=nv_arrayptr(np)) && !ap->fixed)
+	if(np->nvalue.up && nv_isarray(np) && (ap=nv_arrayptr(np)) && !ap->fixed)
 #else
-	else if(np->nvalue.up && nv_isarray(np) && nv_arrayptr(np))
+	if(np->nvalue.up && nv_isarray(np) && nv_arrayptr(np))
 #endif /* SHOPT_FIXEDARRAY */
 		up = np->nvalue.up;
 	if(up && up->cp==Empty)
@@ -1809,7 +1800,7 @@ void nv_putval(Namval_t *np, const char *string, int flags)
 				}
 				if(nv_size(np) <= 1)
 					nv_setsize(np,10);
-				if(nv_isattr(np,NV_INT16P)==NV_INT16P)
+				if(nv_isattr(np,NV_SHORT))
 				{
 					int16_t os=0;
 					if(!up->sp)
@@ -1818,14 +1809,6 @@ void nv_putval(Namval_t *np, const char *string, int flags)
 						os = *(up->sp);
 					*(up->sp) = os+(int16_t)l;
 				}
-				else if(nv_isattr(np,NV_SHORT))
-				{
-					int16_t s=0;
-					if(flags&NV_APPEND)
-						s = *up->sp;
-					*(up->sp) = s+(int16_t)l;
-					nv_onattr(np,NV_NOFREE);
-				}
 				else
 				{
 					int32_t ol=0;
@@ -2443,18 +2426,10 @@ void	_nv_unset(Namval_t *np,int flags)
 		/* called from disc, assign the actual value */
 		nv_local=0;
 	}
-	if(nv_isnonptr(np))
-	{
-		if(nv_isarray(np))
-			np->nvalue.cp = Empty;
-		else
-			np->nvalue.s = 0;
-		goto done;
-	}
 #if SHOPT_FIXEDARRAY
-	else if(np->nvalue.up && nv_isarray(np) && (ap=nv_arrayptr(np)) && !ap->fixed)
+	if(np->nvalue.up && nv_isarray(np) && (ap=nv_arrayptr(np)) && !ap->fixed)
 #else
-	else if(np->nvalue.up && nv_isarray(np) && nv_arrayptr(np))
+	if(np->nvalue.up && nv_isarray(np) && nv_arrayptr(np))
 #endif /* SHOPT_FIXEDARRAY */
 		up = np->nvalue.up;
 	else if(nv_isref(np) && !nv_isattr(np,NV_EXPORT|NV_MINIMAL) && np->nvalue.nrp)
@@ -2711,24 +2686,14 @@ char *nv_getval(Namval_t *np)
 	        	if(nv_isattr (np,NV_LONG))
 				ll = *(Sfulong_t*)up->llp;
 			else if(nv_isattr (np,NV_SHORT))
-			{
-				if(nv_isattr(np,NV_INT16P)==NV_INT16P)
-					ll = *(uint16_t*)(up->sp);
-				else
-					ll = (uint16_t)up->s;
-			}
+				ll = *(uint16_t*)(up->sp);
 			else
 				ll = *(uint32_t*)(up->lp);
 		}
         	else if(nv_isattr (np,NV_LONG))
 			ll = *up->llp;
         	else if(nv_isattr (np,NV_SHORT))
-		{
-			if(nv_isattr(np,NV_INT16P)==NV_INT16P)
-				ll = *up->sp;
-			else
-				ll = up->s;
-		}
+			ll = *up->sp;
         	else
 			ll = *(up->lp);
 		base = nv_size(np);
@@ -2807,12 +2772,7 @@ Sfdouble_t nv_getnum(Namval_t *np)
 			if(nv_isattr(np, NV_LONG))
 				r = (Sflong_t)*((Sfulong_t*)up->llp);
 			else if(nv_isattr(np, NV_SHORT))
-			{
-				if(nv_isattr(np,NV_INT16P)==NV_INT16P)
-					r = (Sflong_t)(*(uint16_t*)up->sp);
-				else
-					r = (Sflong_t)((uint16_t)up->s);
-			}
+				r = (Sflong_t)(*(uint16_t*)up->sp);
 			else
 				r = *((uint32_t*)up->lp);
 		}
@@ -2821,12 +2781,7 @@ Sfdouble_t nv_getnum(Namval_t *np)
 			if(nv_isattr(np, NV_LONG))
 				r = *up->llp;
 			else if(nv_isattr(np, NV_SHORT))
-			{
-				if(nv_isattr(np,NV_INT16P)==NV_INT16P)
-					r = *up->sp;
-				else
-					r = up->s;
-			}
+				r = *up->sp;
 			else
 				r = *up->lp;
 		}

src/cmd/ksh93/sh/nvdisc.c

@@ -845,12 +845,7 @@ static void *num_clone(Namval_t *np, void *val)
 		if(nv_isattr(np,NV_LONG))
 			size = sizeof(Sflong_t);
 		else if(nv_isattr(np,NV_SHORT))
-		{
-			if(nv_isattr(np,NV_INT16P|NV_DOUBLE)==NV_INT16P)
-				size = sizeof(short);
-			else
-				return np->nvalue.ip;
-		}
+			size = sizeof(int16_t);
 		else
 			size = sizeof(int32_t);
 	}

src/cmd/ksh93/sh/nvtype.c

@@ -27,7 +27,7 @@
 #include	"builtins.h"
 
 static const char sh_opttype[] =
-"[-1c?\n@(#)$Id: type (ksh 93u+m) 2021-12-17 $\n]"
+"[-1c?\n@(#)$Id: type (ksh 93u+m) 2024-08-28 $\n]"
 "[--catalog?" SH_DICT "]"
 "[+NAME?\f?\f - set the type of variables to \b\f?\f\b]"
 "[+DESCRIPTION?\b\f?\f\b sets the type on each of the variables specified "
@@ -1122,12 +1122,6 @@ Namval_t *nv_mktype(Namval_t **nodes, int numnodes)
 			{
 				nq->nvalue.cp = pp->data+offset;
 				sp = (char*)np->nvalue.cp;
-				if(nv_isattr(np,NV_INT16P) ==NV_INT16)
-				{
-					sp= (char*)&np->nvalue;
-					nv_onattr(nq,NV_INT16P);
-					j = 1;
-				}
 				if(sp)
 					memcpy((char*)nq->nvalue.cp,sp,dsize);
 				else if(nv_isattr(np,NV_LJUST|NV_RJUST))
@@ -1199,7 +1193,7 @@ Namval_t *nv_mkinttype(char *name, size_t size, int sign, const char *help, Namd
 	mp->nvmeta = (void*)help;
 	nv_onattr(mp,NV_NOFREE|NV_RDONLY|NV_INTEGER|NV_EXPORT);
 	if(size==16)
-		nv_onattr(mp,NV_INT16P);
+		nv_onattr(mp,NV_INT16);
 	else if(size==64)
 		nv_onattr(mp,NV_INT64);
 	if(!sign)

src/cmd/ksh93/sh/parse.c

@@ -2043,7 +2043,8 @@ unsigned long kiaentity(Lex_t *lexp,const char *name,int len,int type,int first,
 	sfputc(sh.stk,'\0');  /* terminate name while writing database output */
 	np = nv_search(stkptr(sh.stk,offset),kia.entity_tree,NV_ADD);
 	stkseek(sh.stk,offset);
-	np->nvalue.i = pkind;
+	np->nvalue.ip = sh_malloc(sizeof(int));
+	*np->nvalue.ip = pkind;
 	nv_setsize(np,width);
 	if(!nv_isattr(np,NV_TAGGED) && first>=0)
 	{
@@ -2064,7 +2065,7 @@ static void kia_add(Namval_t *np, void *data)
 	char *name = nv_name(np);
 	Lex_t	*lp = (Lex_t*)data;
 	NOT_USED(data);
-	kiaentity(lp,name+1,-1,*name,0,-1,(*name=='p'?kia.unknown:kia.script),np->nvalue.i,nv_size(np),"");
+	kiaentity(lp,name+1,-1,*name,0,-1,(*name=='p'?kia.unknown:kia.script),*np->nvalue.ip,nv_size(np),"");
 }
 
 int kiaclose(Lex_t *lexp)

src/cmd/ksh93/sh/subshell.c

@@ -313,7 +313,7 @@ void sh_assignok(Namval_t *np,int add)
 	sh.subshell = 0;
 	mp->nvname = np->nvname;
 	/* Copy value pointers for variables whose values are pointers into the static scope, sh.st */
-	if(!nv_isnonptr(np) && np->nvalue.cp >= (char*)&sh.st && np->nvalue.cp < (char*)&sh.st + sizeof(struct sh_scoped))
+	if(np->nvalue.cp >= (char*)&sh.st && np->nvalue.cp < (char*)&sh.st + sizeof(struct sh_scoped))
 		mp->nvalue = np->nvalue;
 	if(nv_isattr(np,NV_NOFREE))
 		nv_onattr(mp,NV_IDENT);