From 6b9703ffdd9a71c3fb7418ba7744fd734e39f454 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Tue, 6 Apr 2021 05:45:46 +0100 Subject: [PATCH] Backport bugfixes for arrays of 'enum' types from ksh 93v- beta These fixes are applied rather blindly as no one has yet managed to understand the almost entirely uncommented arrays and variables handling code (arrays.c, name.c, nvdisc.c, nvtree.c, nvtype.c). Hopefully we'll figure all that out at some point. In the meantime these backported fixes appear to work fine, and these bugs impact the usability of 'enum', so I'm just going to have to violate my own policy and backport these fixes without understanding them. Thanks to @JohnoKing for putting in a lot of work tracing these. Further discussion at: https://github.com/ksh93/ksh/issues/87 src/cmd/ksh93/sh/array.c: - nv_arraysettype(): * Further simplify the function. After my initial simplification of it (re: 5491fe97), I don't believe there's actually a need to save a duplicate copy of the value. Use the pointer returned by nv_getval() directly to restore the value. * Cope with a null value (nv_getval() returning a NULL pointer). This is needed for compatibility with the backported fix in nvtype.c (below). - array_putval(): If the array's value pointer (up->cp) is a pointer to the empty string, it is set to NULL before calling nv_putv() to prevent an empty string from being deleted. Backport a fix from 93v- that restores the pointer to the empty string if the NV_NOFREE attribute is set. Removing it somehow causes these regressions: enum.sh[86]: ${array[@]} doesn't yield all values for associative enum arrays (expected 'green blue blue red yellow green red orange'; got 'green blue blue yellow green orange') enum.sh[94]: unsetting associative enum array does not work (got 'Color_t -A Colors=([foo]=red [rood]=red)') enum.sh[116]: assigning first enum element to indexed array failed (expected 'red red'; got 'BUG BUG') - nv_associative(): Do not increase the 'nelem' (number of elements) value of the array's 'header' struct if the array is associative and of an enum type. The original 93v- fix only checked for the NV_INTEGER attribute, but backporting that caused several regressions. Using a debug output command I've determined that the exact value of 'type' is somehow consistently set to 0x26 if the array is associative and of an enum type, which is NV_INTEGER | NV_LTOU | NV_RJUST as defined in include/nval.h. I cannot find where/how that value is determined. In any case this fix, based on but more specific than the 93v- one, appears to work fine. Removing it somehow causes this regression: enum.sh[94]: unsetting associative enum array does not work (got 'Color_t -A Colors=()') src/cmd/ksh93/sh/nvtype.c: nv_settype(): - Another fix backported from 93v-. If the variable is an array, also set the type of element 0 of that array using a call to nv_arraysettype(). The value may be null. Removing this somehow causes this regression: enum.sh[94]: unsetting associative enum array does not work (got 'Color_t -A Colors=()') src/cmd/ksh93/tests/enum.sh: - Add tests for all the bugs fixed here, plus some hypothetical bugs (e.g., do the same tests for indexed enum type arrays as for associative enum type arrays, even though indexed enum type arrays didn't have all the same problems). Co-authored-by: Johnothan King Resolves: https://github.com/ksh93/ksh/issues/87 --- NEWS | 7 ++++++ src/cmd/ksh93/sh/array.c | 19 ++++++++++------ src/cmd/ksh93/sh/nvtype.c | 2 +- src/cmd/ksh93/tests/enum.sh | 43 ++++++++++++++++++++++++++++++++++++- 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/NEWS b/NEWS index 8ec9ee5fa..6a7849c95 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,13 @@ Any uppercase BUG_* names are modernish shell bug IDs. like 'unset arr[3]' to unset not just element 3 of the array but all elements starting from 3, if a range expansion like ${arr[5..10]} was previously used. +- Several fixes for arrays of a type created by 'enum' were backported from ksh + 93v-, further to the two enum array fixes already applied on 2021-02-01: + 1. The array[@]} expansion was fixed for associative arrays of an enum type. + 2. Assignments now work correctly for all enum values for both indexed and + associative arrays. + 3. 'unset' will now completely unset an associative array of an enum type. + 2021-04-04: - A bug was fixed that caused a broken prompt display upon redrawing the diff --git a/src/cmd/ksh93/sh/array.c b/src/cmd/ksh93/sh/array.c index 59cea1abe..5c92765fc 100644 --- a/src/cmd/ksh93/sh/array.c +++ b/src/cmd/ksh93/sh/array.c @@ -421,18 +421,17 @@ int nv_arraysettype(Namval_t *np, Namval_t *tp, const char *sub, int flags) } if(nq = nv_search(sub, ap->table, NV_ADD)) { - char *saved_value; - int rdonly = nv_isattr(np,NV_RDONLY); + char *saved_value = NIL(char*); if(!nq->nvfun && nq->nvalue.cp && *nq->nvalue.cp==0) _nv_unset(nq,NV_RDONLY); nv_arraychild(np,nq,0); if(!nv_isattr(tp,NV_BINARY)) - saved_value = stkcopy(stkstd,nv_getval(np)); + saved_value = nv_getval(np); if(!nv_clone(tp,nq,flags|NV_NOFREE)) return(0); - if(!rdonly) + if(!nv_isattr(np,NV_RDONLY)) nv_offattr(nq,NV_RDONLY); - if(!nv_isattr(tp,NV_BINARY)) + if(saved_value) nv_putval(nq,saved_value,0); ap->nelem |= ARRAY_SCAN; return(1); @@ -699,6 +698,8 @@ static void array_putval(Namval_t *np, const char *string, int flags, Namfun_t * #endif /* SHOPT_FIXEDARRAY */ np->nvalue.up = up; nv_putv(np,string,flags,&ap->hdr); + if(nofree && !up->cp) + up->cp = Empty; #if SHOPT_FIXEDARRAY if(fp = (struct fixed_array*)ap->fixed) { @@ -1775,7 +1776,13 @@ void *nv_associative(register Namval_t *np,const char *sp,int mode) nv_arraychild(np,mp,0); if(sh.subshell) np = sh_assignok(np,1); - if(!ap->header.scope || !nv_search(sp,dtvnext(ap->header.table),0)) + /* + * type == 0x26 == NV_INTEGER|NV_LTOU|NV_RJUST (see include/nval.h) indicates an + * associative array of a type created by the enum command. nelem should not be + * increased in that case or 'unset' will fail to completely unset such an array. + */ + if(type != (NV_INTEGER|NV_LTOU|NV_RJUST) + && (!ap->header.scope || !nv_search(sp,dtvnext(ap->header.table),0))) ap->header.nelem++; if(nv_isnull(mp)) { diff --git a/src/cmd/ksh93/sh/nvtype.c b/src/cmd/ksh93/sh/nvtype.c index a842ead72..f3c3908ce 100644 --- a/src/cmd/ksh93/sh/nvtype.c +++ b/src/cmd/ksh93/sh/nvtype.c @@ -1332,7 +1332,7 @@ int nv_settype(Namval_t* np, Namval_t *tp, int flags) nv_putsub(np,"0",ARRAY_FILL); ap = nv_arrayptr(np); nelem = 1; - + nv_arraysettype(np,tp,"0",flags); } } else diff --git a/src/cmd/ksh93/tests/enum.sh b/src/cmd/ksh93/tests/enum.sh index f2cb142c9..8c632cc5b 100755 --- a/src/cmd/ksh93/tests/enum.sh +++ b/src/cmd/ksh93/tests/enum.sh @@ -64,8 +64,11 @@ arr[green]=foo read -A arr <<< 'x y z xx yy' [[ ${arr[1]} == ${arr[green]} ]] || err_exit 'arr[1] != arr[green] after read' -# enum arrays didn't block unspecified values +# ====== +# Various fixed bugs with associative and indexed arrays of a type created by 'enum' # https://github.com/ksh93/ksh/issues/87 + +# values not specified by the enum type should be blocked exp=': Color_t: clr[2]: invalid value WRONG' got=$(set +x; redirect 2>&1; Color_t -a clr=(red blue WRONG yellow); printf '%s\n' "${clr[@]}") (((e = $?) == 1)) && [[ $got == *"$exp" ]] || err_exit "indexed enum array, unspecified value:" \ @@ -75,5 +78,43 @@ got=$(set +x; redirect 2>&1; Color_t -A clr=([foo]=red [bar]=blue [bad]=BAD); pr (((e = $?) == 1)) && [[ $got == *"$exp" ]] || err_exit "associative enum array, unspecified value:" \ "expected status 1, *$(printf %q "$exp"); got status $e, $(printf %q "$got")" +# associative enum array +# (need 'eval' to delay parsing when testing with shcomp, as it parses the entire script without executing the type definition) +eval 'Color_t -A Colors=([foo]=red [bar]=blue [bad]=green [zut]=orange [blauw]=blue [rood]=red [groen]=green [geel]=yellow)' +exp='green blue blue red yellow green red orange' +got=${Colors[@]} +[[ $got == "$exp" ]] || err_exit "\${array[@]} doesn't yield all values for associative enum arrays" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" +exp='Color_t -A Colors=([bad]=green [bar]=blue [blauw]=blue [foo]=red [geel]=yellow [groen]=green [rood]=red [zut]=orange)' +got=$(typeset -p Colors) +[[ $got == "$exp" ]] || err_exit "'typeset -p' doesn't yield all values for associative enum arrays" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" +unset Colors +got=$(typeset -p Colors) +[[ -n $got ]] && err_exit "unsetting associative enum array does not work (got $(printf %q "$got"))" + +# indexed enum array +# (need 'eval' to delay parsing when testing with shcomp, as it parses the entire script without executing the type definition) +eval 'Color_t -a iColors=(red blue green orange blue red green yellow)' +exp='red blue green orange blue red green yellow' +got=${iColors[@]} +[[ $got == "$exp" ]] || err_exit "\${array[@]} doesn't yield all values for indexed enum arrays" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" +exp='Color_t -a iColors=(red blue green orange blue red green yellow)' +got=$(typeset -p iColors) +[[ $got == "$exp" ]] || err_exit "'typeset -p' doesn't yield all values for indexed enum arrays" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" +unset iColors +got=$(typeset -p iColors) +[[ -n $got ]] && err_exit "unsetting indexed enum array does not work (got $(printf %q "$got"))" + +# assigning the first enum type element should work +Color_t -a testarray +testarray[3]=red +exp="red red" +got="${testarray[3]:-BUG} ${testarray[@]:-BUG}" +[[ $got == "$exp" ]] || err_exit "assigning first enum element to indexed array failed" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" + # ====== exit $((Errors<125?Errors:125))