Skip to content

Commit

Permalink
Revert "[v1.0] Remove experimental .getn discipline"
Browse files Browse the repository at this point in the history 
This reverts commit 42fc5c4.
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.
  • Loading branch information
@McDutchie
McDutchie committed Aug 18, 2022
1 parent 110fc45 commit 274331a
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 16 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/data/variables.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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[] =
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
View file
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.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. */
Expand Down
60 changes: 46 additions & 14 deletions src/cmd/ksh93/sh/nvdisc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -374,15 +375,15 @@ static void assign(Namval_t *np,const char* val,int flags,Namfun_t *handle)
* This function executes a lookup disc and then performs
* the lookup on the given node <np>
*/
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;
Expand All @@ -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);
Expand All @@ -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))
Expand All @@ -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)
Expand All @@ -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 <event> to <action>
* If action==np, the current disc is returned
Expand Down Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions src/cmd/ksh93/tests/variables.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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))

0 comments on commit 274331a

Please sign in to comment.