Skip to content

Commit

Permalink
nvalue sanitisation part 4 (re: 0686c70, 0ebaab3, 6cdc2dc)
Browse files Browse the repository at this point in the history
This commit takes a large first step towards getting rid of 'union
Value' altogether, by turning the nvalue member of the Namval_t
struct into a void pointer instead of a union Value pointer.
(Also, it is now defined entirely in nval.h, eliminating the
_NV_PRIVATE part from name.h).

So, for all those Namval_t* pointers such as np, instead of
np->nvalue.cp, np->nvalue.lp, etc., we now simply use np->nvalue.

The code everywhere is correspondingly updated to do local
typecasts where the nvalue pointer is dereferenced. Sometimes,
explicit typecasts seemed the most practical, other times I've
preferred implicit typecasts through assigning np->nvalue to an
appropriately typed local pointer variable and dereferencing that.

On the other hand, this change has alllowed getting rid of many
explicit typecasts as well, as C90 void pointers are typeless and
cna be implicitly typecast to any type.

In my view, this change is already increasing the consistency,
legibility and transparency of the name-value handling in the code.
When I was new to this code base (and still pretty new to C), I
found the nvalue union usage one of the most mystifying aspects.
The code now shows more explicitly what is being done.
  • Loading branch information
McDutchie committed Nov 29, 2024
1 parent 6cdc2dc commit 70a0032
Show file tree
Hide file tree
Showing 25 changed files with 365 additions and 345 deletions.
4 changes: 2 additions & 2 deletions src/cmd/ksh93/bltins/cd_pwd.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
*/
static void rehash(Namval_t *np,void *data)
{
Pathcomp_t *pp = (Pathcomp_t*)np->nvalue.cp;
Pathcomp_t *pp = np->nvalue;
if(pp && *pp->name!='/')
nv_rehash(np,data);
}
Expand Down Expand Up @@ -135,7 +135,7 @@ int b_cd(int argc, char *argv[],Shbltin_t *context)
&& !(dir[0]=='.' && (dir[1]=='/' || dir[1]==0))
&& !(dir[0]=='.' && dir[1]=='.' && (dir[2]=='/' || dir[2]==0)))
{
if((dp=sh_scoped(CDPNOD)->nvalue.cp) && !(cdpath = (Pathcomp_t*)sh.cdpathlist))
if((dp=sh_scoped(CDPNOD)->nvalue) && !(cdpath = (Pathcomp_t*)sh.cdpathlist))
{
if(cdpath=path_addpath(NULL,dp,PATH_CDPATH))
sh.cdpathlist = cdpath;
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/ksh93/bltins/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,10 @@ int b_dot_cmd(int n,char *argv[],Shbltin_t *context)
np = nv_search(script,sh.fun_tree,0);
if(np && is_afunction(np) && !nv_isattr(np,NV_FPOSIX) && !(sh_isoption(SH_POSIX) && context->bnode==SYSDOT))
{
if(!np->nvalue.ip)
if(!np->nvalue)
{
path_search(script,NULL,0);
if(np->nvalue.ip)
if(np->nvalue)
{
if(nv_isattr(np,NV_FPOSIX))
np = 0;
Expand Down Expand Up @@ -307,7 +307,7 @@ int b_dot_cmd(int n,char *argv[],Shbltin_t *context)
prevscope->save_tree = sh.var_tree;
tofree = sh.st.filename;
if(np)
sh.st.filename = np->nvalue.rp->fname;
sh.st.filename = ((struct Ufunction*)np->nvalue)->fname;
nv_putval(SH_PATHNAMENOD, sh.st.filename ,NV_NOFREE);
sh.posix_fun = 0;
if(np || argv[1])
Expand Down
10 changes: 5 additions & 5 deletions src/cmd/ksh93/bltins/print.c
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ static ssize_t fmtbase64(Sfio_t *iop, char *string, int alt)
nv_offattr(np,NV_RAW);
}
else
cp = (char*)np->nvalue.cp;
cp = np->nvalue;
if((size = n)==0)
size = strlen(cp);
size = sfwrite(iop, cp, size);
Expand Down Expand Up @@ -844,15 +844,15 @@ static int extend(Sfio_t* sp, void* v, Sffmt_t* fe)
np = nv_open(argp,sh.var_tree,NV_VARNAME|NV_NOARRAY);
_nv_unset(np,0);
nv_onattr(np,NV_INTEGER);
if (np->nvalue.lp = new_of(int32_t,0))
*np->nvalue.lp = 0;
if (np->nvalue = new_of(int32_t,0))
*((int32_t*)np->nvalue) = 0;
nv_setsize(np,10);
if(sizeof(int)==sizeof(int32_t))
value->ip = (int*)np->nvalue.lp;
value->ip = np->nvalue;
else
{
int32_t sl = 1;
value->ip = (int*)(((char*)np->nvalue.lp) + (*((char*)&sl) ? 0 : sizeof(int)));
value->ip = (int*)(((char*)np->nvalue) + (*((char*)&sl) ? 0 : sizeof(int)));
}
break;
}
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/bltins/read.c
Original file line number Diff line number Diff line change
Expand Up @@ -487,8 +487,8 @@ int sh_readline(char **names, volatile int fd, int flags, ssize_t size, Sflong_t
#else
int optimize = 1;
#endif
if(optimize && c==size && np->nvalue.cp && !nv_isarray(np))
memcpy((char*)np->nvalue.cp,var,c);
if(optimize && c==size && np->nvalue && !nv_isarray(np))
memcpy(np->nvalue,var,c);
else
{
Namval_t *mp;
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ int test_unop(int op,const char *arg)
return isref;
if(isref)
{
if(np->nvalue.cp)
if(np->nvalue)
np = nv_refnode(np);
else
return 0;
Expand Down
32 changes: 19 additions & 13 deletions src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,10 @@ int b_typeset(int argc,char *argv[],Shbltin_t *context)
if(flag&NV_TYPE)
{
Stk_t *stkp = sh.stk;
int off=0,offset = stktell(stkp);
#if SHOPT_NAMESPACE
int off = 0;
#endif /* SHOPT_NAMESPACE */
int offset = stktell(stkp);
if(!tdata.prefix)
return sh_outtype(sfstdout);
sfputr(stkp,NV_CLASS,-1);
Expand Down Expand Up @@ -648,7 +651,9 @@ static int setall(char **argv,int flag,Dt_t *troot,struct tdata *tp)
int nvflags=(flag&(NV_ARRAY|NV_NOARRAY|NV_VARNAME|NV_IDENT|NV_ASSIGN|NV_STATIC|NV_MOVE));
int r=0, ref=0, comvar=(flag&NV_COMVAR),iarray=(flag&NV_IARRAY);
Dt_t *save_vartree = NULL;
#if SHOPT_NAMESPACE
Namval_t *save_namespace = NULL;
#endif
if(flag&NV_GLOBAL)
{
save_vartree = sh.var_tree;
Expand Down Expand Up @@ -757,8 +762,8 @@ static int setall(char **argv,int flag,Dt_t *troot,struct tdata *tp)
np = nv_search(stkptr(sh.stk,offset),troot,0);
stkseek(sh.stk,offset);
}
if(np && np->nvalue.cp)
np->nvalue.rp->help = tp->help;
if(np && np->nvalue)
((struct Ufunction*)np->nvalue)->help = tp->help;
}
continue;
}
Expand Down Expand Up @@ -1420,7 +1425,7 @@ static int unall(int argc, char **argv, Dt_t *troot)
{
if(troot!=sh.fun_base)
np->nvflag = 0; /* invalidate */
else if(!(np->nvalue.rp && np->nvalue.rp->running))
else if(!(np->nvalue && ((struct Ufunction*)np->nvalue)->running))
nv_delete(np,troot,0);
}
/* The alias has been unset by call to _nv_unset, remove it from the tree */
Expand Down Expand Up @@ -1488,9 +1493,10 @@ static int print_namval(Sfio_t *file,Namval_t *np,int flag, struct tdata *tp)
if(isfun)
{
char *fname=0;
struct Ufunction *rp = np->nvalue;
if(nv_isattr(np,NV_NOFREE))
return 0;
if(!flag && !np->nvalue.ip)
if(!flag && !rp)
sfputr(file,"typeset -fu",' ');
else if(!flag && !nv_isattr(np,NV_FPOSIX))
sfputr(file,"function",' ');
Expand All @@ -1500,28 +1506,28 @@ static int print_namval(Sfio_t *file,Namval_t *np,int flag, struct tdata *tp)
sfputr(file,cp,-1);
if(nv_isattr(np,NV_FPOSIX))
sfwrite(file,"()",2);
else if(np->nvalue.rp)
else if(rp)
{
int i;
/* output function reference list (for .sh.math.* functions) */
for(i = 0; i < np->nvalue.rp->argc; i++)
sfprintf(file," %s",np->nvalue.rp->argv[i]);
for(i = 0; i < rp->argc; i++)
sfprintf(file," %s",rp->argv[i]);
}
if(np->nvalue.rp && nv_funtree(np))
fname = np->nvalue.rp->fname;
if(rp && rp->ptree)
fname = rp->fname;
else
flag = '\n';
if(flag)
{
if(tp->pflag && np->nvalue.rp && nv_funtree(np))
sfprintf(file," #line %d %s\n", np->nvalue.rp->lineno, fname ? sh_fmtq(fname) : Empty);
if(tp->pflag && rp && rp->ptree)
sfprintf(file," #line %d %s\n", rp->lineno, fname ? sh_fmtq(fname) : Empty);
else
sfputc(file, '\n');
}
else
{
sfputc(file, '\n');
sh_deparse(file, (Shnode_t*)(nv_funtree(np)), 2 | nv_isattr(np,NV_FPOSIX), 0);
sh_deparse(file, (Shnode_t*)(rp->ptree), 2 | nv_isattr(np,NV_FPOSIX), 0);
}
return nv_size(np) + 1;
}
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/edit/history.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ int sh_histinit(void)
hp = hist_trim(hp,(int)hp->histind-maxlines);
}
sfdisc(hp->histfp,&hp->histdisc);
(HISTCUR)->nvalue.lp = (&hp->histind);
HISTCUR->nvalue = &hp->histind;
sh_timeradd(1000L*(HIST_RECENT-30), 1, hist_touch, hp->histname);
#if SHOPT_ACCTFILE
if(sh_isstate(SH_INTERACTIVE))
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/ksh93/features/externs
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ tst note{ determining size of PID variables }end output{
* If there is ever a system where this test returns 1, then both this
* test and nv_getval() must learn a new integer size attribute.
*/
if(s==sizeof(int16_t)) /* union Value: sp */
if(s==sizeof(int16_t))
a="NV_INTEGER|NV_SHORT";
else if(s==sizeof(int32_t)) /* union Value: lp */
else if(s==sizeof(int32_t))
a="NV_INTEGER";
else if(s==sizeof(Sflong_t)) /* union Value: llp */
else if(s==sizeof(Sflong_t))
a="NV_INTEGER|NV_LONG";
else
return 1;
Expand Down
26 changes: 11 additions & 15 deletions src/cmd/ksh93/include/name.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,12 @@
* hyenias <[email protected]> *
* *
***********************************************************************/
#ifndef _NV_PRIVATE
#ifndef name_h_defined
#define name_h_defined
/*
* This is the implementation header file for name-value pairs
*/

#define _NV_PRIVATE \
Namfun_t *nvfun; /* pointer to trap functions */ \
union Value nvalue; /* value field */ \
void *nvmeta; /* pointer to any of various kinds of type-dependent data */

#include <ast.h>
#include <cdt.h>

Expand Down Expand Up @@ -135,21 +131,21 @@ struct Ufunction
#define nv_isref(n) (nv_isattr((n),NV_REF|NV_TAGGED|NV_FUNCT)==NV_REF)
#define is_abuiltin(n) (nv_isattr(n,NV_BLTIN|NV_INTEGER)==NV_BLTIN)
#define is_afunction(n) (nv_isattr(n,NV_FUNCTION|NV_REF)==NV_FUNCTION)
#define nv_funtree(n) ((n)->nvalue.rp->ptree)
#define nv_funtree(n) (((struct Ufunction*)(n)->nvalue)->ptree)

/* NAMNOD MACROS */
/* ... for attributes */

#define nv_setattr(n,f) ((n)->nvflag = (f))
#define nv_context(n) ((void*)(n)->nvfun) /* for builtins */
/* The following are for name references */
#define nv_refnode(n) ((n)->nvalue.nrp->np)
#define nv_reftree(n) ((n)->nvalue.nrp->root)
#define nv_reftable(n) ((n)->nvalue.nrp->table)
#define nv_refsub(n) ((n)->nvalue.nrp->sub)
#define nv_refnode(n) (((struct Namref*)(n)->nvalue)->np)
#define nv_reftree(n) (((struct Namref*)(n)->nvalue)->root)
#define nv_reftable(n) (((struct Namref*)(n)->nvalue)->table)
#define nv_refsub(n) (((struct Namref*)(n)->nvalue)->sub)
#if SHOPT_FIXEDARRAY
# define nv_refindex(n) ((n)->nvalue.nrp->curi)
# define nv_refdimen(n) ((n)->nvalue.nrp->dim)
# define nv_refindex(n) (((struct Namref*)(n)->nvalue)->curi)
# define nv_refdimen(n) (((struct Namref*)(n)->nvalue)->dim)
#endif /* SHOPT_FIXEDARRAY */

/* ... etc */
Expand All @@ -158,7 +154,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_hasget(np))
#define nv_isnull(np) (!(np)->nvalue && !_nv_hasget(np))

/* ... for arrays */

Expand Down Expand Up @@ -262,4 +258,4 @@ extern const char e_typecompat[];
extern const char e_globalref[];
extern const char e_tolower[];
extern const char e_toupper[];
#endif /* _NV_PRIVATE */
#endif /* name_h_defined */
10 changes: 3 additions & 7 deletions src/cmd/ksh93/include/nval.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,9 @@ struct Namval
unsigned short nvflag; /* attributes */
unsigned short nvsize; /* size or base */
#endif
#ifdef _NV_PRIVATE
_NV_PRIVATE
#else
Namfun_t *nvfun;
char *nvalue;
char *nvprivate;
#endif /* _NV_PRIVATE */
Namfun_t *nvfun; /* pointer to trap functions */
void *nvalue; /* pointer to any kind of value */
void *nvmeta; /* pointer to any of various kinds of type-dependent data */
};

#define NV_CLASS ".sh.type"
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/shell.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ typedef struct Shell_s Shell_t;
#include <shcmd.h>

/* get pointer to a built-in command's entry function */
#define funptr(n) ((Shbltin_f)(n)->nvalue.bfp)
#define funptr(n) ((Shbltin_f)(n)->nvalue)

typedef void (*Shinit_f)(Shell_t*, int);
#ifndef SH_wait_f_defined
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-11-28" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-11-29" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
13 changes: 7 additions & 6 deletions src/cmd/ksh93/sh/arith.c
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ static Namval_t *scope(Namval_t *np,struct lval *lvalue,int assign)
&& sh_macfun(cp, offset = stktell(sh.stk)))
{
Fun = sh_arith(sub=stkptr(sh.stk,offset));
FunNode.nvalue.ldp = &Fun;
FunNode.nvalue = &Fun;
nv_onattr(&FunNode,NV_NOFREE|NV_LDOUBLE|NV_RDONLY);
cp[flag] = c;
return &FunNode;
Expand Down Expand Up @@ -330,9 +330,10 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
stkseek(sh.stk,off);
if(np=nv_search(stkptr(sh.stk,off),sh.fun_tree,0))
{
lvalue->nargs = -np->nvalue.rp->argc;
lvalue->fun = (Math_f)np;
break;
struct Ufunction *rp = np->nvalue;
lvalue->nargs = -rp->argc;
lvalue->fun = (Math_f)np;
break;
}
if(fsize<=(sizeof(tp->fname)-2))
lvalue->fun = (Math_f)sh_mathstdfun(*ptr,fsize,&lvalue->nargs);
Expand Down Expand Up @@ -385,14 +386,14 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
if(!sh_isoption(SH_POSIX) && (cp[0] == 'i' || cp[0] == 'I') && (cp[1] == 'n' || cp[1] == 'N') && (cp[2] == 'f' || cp[2] == 'F') && cp[3] == 0)
{
Inf = strtold("Inf", NULL);
Infnod.nvalue.ldp = &Inf;
Infnod.nvalue = &Inf;
np = &Infnod;
nv_onattr(np,NV_NOFREE|NV_LDOUBLE|NV_RDONLY);
}
else if(!sh_isoption(SH_POSIX) && (cp[0] == 'n' || cp[0] == 'N') && (cp[1] == 'a' || cp[1] == 'A') && (cp[2] == 'n' || cp[2] == 'N') && cp[3] == 0)
{
NaN = strtold("NaN", NULL);
NaNnod.nvalue.ldp = &NaN;
NaNnod.nvalue = &NaN;
np = &NaNnod;
nv_onattr(np,NV_NOFREE|NV_LDOUBLE|NV_RDONLY);
}
Expand Down
Loading

0 comments on commit 70a0032

Please sign in to comment.