From 274331acac35674e4905a795262b9bbf58d17624 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 18 Aug 2022 19:49:55 +0100 Subject: [PATCH] Revert "[v1.0] Remove experimental .getn discipline" This reverts commit 42fc5c4c0da82b4427c34f2e87a76e0171e560b4. It somehow caused the following reproducer to fail. Reason unknown. typeset -i x function x.set { :; } x[0]=0 unset x typeset -p x Expected output: none Actual output: typeset -a -i x=() The 'x' variable fails to be unset. With the .get and .getn discipline functions instead of .set, the above reproducer still fails even after the revert, but that always has failed, back to at least 93u+ 2012-08-01. This is yet more evidence that discipline functions are a mess. But a proper cleanup will require a lot of time and very thorough testing, so it will have to wait until some future release. src/cmd/ksh93/tests/variables.sh: - In addition to the revert, add the above reproducer as a regression test for the .set, .unset and .append disciplines. --- src/cmd/ksh93/data/variables.c | 2 +- src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/nvdisc.c | 60 ++++++++++++++++++++++++-------- src/cmd/ksh93/tests/variables.sh | 25 +++++++++++++ 4 files changed, 73 insertions(+), 16 deletions(-) diff --git a/src/cmd/ksh93/data/variables.c b/src/cmd/ksh93/data/variables.c index 61467aa69..b9dfb6136 100644 --- a/src/cmd/ksh93/data/variables.c +++ b/src/cmd/ksh93/data/variables.c @@ -104,7 +104,7 @@ const struct shtable2 shtab_variables[] = "", 0, (char*)0 }; -const char *nv_discnames[] = { "get", "set", "append", "unset", 0 }; +const char *nv_discnames[] = { "get", "set", "append", "unset", "getn", 0 }; #if SHOPT_STATS const Shtable_t shtab_stats[] = diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 65fc12eea..48c678993 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -18,7 +18,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.3-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2022-08-16" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2022-08-18" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2022 Contributors to ksh " SH_RELEASE_FORK /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */ diff --git a/src/cmd/ksh93/sh/nvdisc.c b/src/cmd/ksh93/sh/nvdisc.c index ab874e487..194f38de3 100644 --- a/src/cmd/ksh93/sh/nvdisc.c +++ b/src/cmd/ksh93/sh/nvdisc.c @@ -151,15 +151,16 @@ void nv_putv(Namval_t *np, const char *value, int flags, register Namfun_t *nfp) } } -#define LOOKUP 0 +#define LOOKUPS 0 #define ASSIGN 1 #define APPEND 2 #define UNASSIGN 3 +#define LOOKUPN 4 struct vardisc { Namfun_t fun; - Namval_t *disc[4]; + Namval_t *disc[5]; }; struct blocked @@ -286,8 +287,8 @@ static void assign(Namval_t *np,const char* val,int flags,Namfun_t *handle) savelex = *lexp; sh_lexopen(lexp, 0); /* needs full init (0), not what it calls reinit (1) */ block(bp,type); - if(bflag = (type==APPEND && !isblocked(bp,LOOKUP))) - block(bp,LOOKUP); + if(bflag = (type==APPEND && !isblocked(bp,LOOKUPS))) + block(bp,LOOKUPS); sh_pushcontext(&checkpoint, 1); jmpval = sigsetjmp(checkpoint.buff, 0); if(!jmpval) @@ -297,7 +298,7 @@ static void assign(Namval_t *np,const char* val,int flags,Namfun_t *handle) sh_iorestore(checkpoint.topfd, jmpval); unblock(bp,type); if(bflag) - unblock(bp,LOOKUP); + unblock(bp,LOOKUPS); if(!vp->disc[type]) chktfree(np,vp); *lexp = savelex; @@ -374,15 +375,15 @@ done: * This function executes a lookup disc and then performs * the lookup on the given node */ -static char* lookup(Namval_t *np, Namfun_t *handle) +static char* lookup(Namval_t *np, int type, Sfdouble_t *dp,Namfun_t *handle) { register struct vardisc *vp = (struct vardisc*)handle; struct blocked block, *bp = block_info(np, &block); - register Namval_t *nq = vp->disc[LOOKUP]; + register Namval_t *nq = vp->disc[type]; register char *cp=0; Namval_t node; union Value *up = np->nvalue.up; - if(nq && !isblocked(bp,LOOKUP)) + if(nq && !isblocked(bp,type)) { struct checkpt checkpoint; int jmpval; @@ -397,7 +398,12 @@ static char* lookup(Namval_t *np, Namfun_t *handle) nv_onattr(SH_VALNOD,NV_NOFREE); _nv_unset(SH_VALNOD,0); } - block(bp,LOOKUP); + if(type==LOOKUPN) + { + nv_onattr(SH_VALNOD,NV_DOUBLE|NV_INTEGER); + nv_setsize(SH_VALNOD,10); + } + block(bp,type); block(bp, UNASSIGN); /* make sure nv_setdisc doesn't invalidate 'vp' by freeing it */ sh_pushcontext(&checkpoint, 1); jmpval = sigsetjmp(checkpoint.buff, 0); @@ -407,10 +413,15 @@ static char* lookup(Namval_t *np, Namfun_t *handle) if(sh.topfd != checkpoint.topfd) sh_iorestore(checkpoint.topfd, jmpval); unblock(bp,UNASSIGN); - unblock(bp,LOOKUP); - if(!vp->disc[LOOKUP]) + unblock(bp,type); + if(!vp->disc[type]) chktfree(np,vp); - if(cp = nv_getval(SH_VALNOD)) + if(type==LOOKUPN) + { + cp = (char*)(SH_VALNOD->nvalue.cp); + *dp = nv_getnum(SH_VALNOD); + } + else if(cp = nv_getval(SH_VALNOD)) cp = stkcopy(stkstd,cp); _nv_unset(SH_VALNOD,NV_RDONLY); if(!nv_isnull(&node)) @@ -424,7 +435,12 @@ static char* lookup(Namval_t *np, Namfun_t *handle) if(nv_isarray(np)) np->nvalue.up = up; if(!cp) - cp = nv_getv(np,handle); + { + if(type==LOOKUPS) + cp = nv_getv(np,handle); + else + *dp = nv_getn(np,handle); + } if(bp== &block) block_done(bp); if(nq && nq->nvalue.rp && nq->nvalue.rp->running==1) @@ -435,6 +451,19 @@ static char* lookup(Namval_t *np, Namfun_t *handle) return(cp); } +static char* lookups(Namval_t *np, Namfun_t *handle) +{ + return(lookup(np,LOOKUPS,(Sfdouble_t*)0,handle)); +} + +static Sfdouble_t lookupn(Namval_t *np, Namfun_t *handle) +{ + Sfdouble_t d; + lookup(np,LOOKUPN, &d ,handle); + return(d); +} + + /* * Set disc on given to * If action==np, the current disc is returned @@ -531,7 +560,10 @@ char *nv_setdisc(register Namval_t* np,register const char *event,Namval_t *acti else if(action) { Namdisc_t *dp = (Namdisc_t*)vp->fun.disc; - dp->getval = lookup; + if(type==LOOKUPS) + dp->getval = lookups; + else if(type==LOOKUPN) + dp->getnum = lookupn; vp->disc[type] = action; } else diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index 6dace72dc..e6abcbeef 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -1381,6 +1381,14 @@ got=$("$SHELL" -c $'foo=baz; foo+=_foo "$SHELL" -c \'print $foo\'; print $foo') [[ $exp == "$got" ]] || err_exit "using the += operator for invocation-local assignments changes variables outside of the invocation-local scope" \ "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# ====== +# Crash when setting attribute after getn (numeric get) discipline +# https://github.com/ksh93/ksh/issues/435#issuecomment-1148813866 +got=$("$SHELL" -c 'foo.getn() { .sh.value=2.3*4.5; }; typeset -F2 foo; typeset -p foo' 2>&1) +exp='typeset -F 2 foo=10.35' +[[ $got == "$exp" ]] || err_exit "Setting attribute after setting getn discipline fails" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" + # ====== # As of 2022-07-12, the current scope is restored after changing .sh.level in a DEBUG trap exp=$'a: 2 CHILD\nb: 1 PARENT\nc: 2 CHILD\nd: 1 PARENT' @@ -1435,5 +1443,22 @@ got=$var1 unset var1 var2 [[ $got == one ]] || err_exit ".sh.value not restored after second .get discipline call (got $(printf %q "$got"))" +# ====== +# TODO: this is known to fail with a .get or .getn discipline function +for disc in set append unset +do + for type in i F E + do + got=$(eval " + typeset -$type x + function x.$disc { :; } + x[0]=0 + unset x + typeset -p x + ") + [[ -z $got ]] || err_exit "-$type array with .$disc discipline fails to be unset (got $(printf %q "$got"))" + done +done + # ====== exit $((Errors<125?Errors:125))