From db2b1affdfb01e7d447fd2062872c72a9f8fa693 Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Mon, 5 Apr 2021 22:11:23 +0100 Subject: [PATCH] Fix unsetting array element after expanding array subscript range Simple reproducer: set -A arr a b c d; : ${arr[1..2]}; unset arr[1]; echo ${arr[@]} Output: a Expected output: a c d The ${arr[1..2]} expansion broke the subsequent 'unset' command so that it unsets element 1 and on, instead of only 1. This regression was introduced in nv_endsubscript() on 2009-07-31: https://github.com/ksh93/ast-open-history/commit/c47896b4/src/cmd/ksh93/sh/array.c That change checks for the ARRAY_SCAN attribute which enables processing ranges of array elements instead of single array elements, and restores it after. That restore is evidently not correct as it causes the subsequent unset command to malfunction. If we revert that change, the bug disappears and the regression tests show no failures. However, I don't know what this was meant to accomplish and what other bug we might introduce by reverting this. However, no corresponding regression test was added along with the 2009-07-31 change, nor is there any corresponding message in the changelog. So this looks to be one of those mystery changes that we'll never know the reason for. Since we currently have proof that this change causes breakage and no evidence that it fixes anything, I'll go ahead and revert it (and add a regression test, of course). If that causes another regression, hopefully someone will find it at some point. src/cmd/ksh93/sh/array.c: nv_endsubscript(): - Revert the 2009-07-31 change that saves/restores the ARRAY_SCAN attribute. - Keep the 'ap' pointer as it is now used by newer code. Move the declaration up to the beginning of the block, as is customary. src/cmd/ksh93/sh/init.c: - Cosmetic change: remove an unused array_scan() macro that I found when grepping the code for ARRAY_SCAN. The macro was introduced in version 2001-06-01 but the code that used it was replaced in version 2001-07-04, without removing the macro itself. Resolves: https://github.com/ksh93/ksh/issues/254 --- NEWS | 6 ++++++ src/cmd/ksh93/include/version.h | 2 +- src/cmd/ksh93/sh/array.c | 8 +------- src/cmd/ksh93/sh/init.c | 2 -- src/cmd/ksh93/tests/arrays.sh | 19 +++++++++++++++++++ 5 files changed, 27 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 4f8d9d91b..8ec9ee5fa 100644 --- a/NEWS +++ b/NEWS @@ -3,6 +3,12 @@ For full details, see the git log at: https://github.com/ksh93/ksh Any uppercase BUG_* names are modernish shell bug IDs. +2021-04-05: + +- Fixed a regression, introduced in ksh 93t+ 2009-07-31, that caused a command + 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. + 2021-04-04: - A bug was fixed that caused a broken prompt display upon redrawing the diff --git a/src/cmd/ksh93/include/version.h b/src/cmd/ksh93/include/version.h index 01cbcd8a7..1a8983bce 100644 --- a/src/cmd/ksh93/include/version.h +++ b/src/cmd/ksh93/include/version.h @@ -20,7 +20,7 @@ #define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */ #define SH_RELEASE_SVER "1.0.0-alpha" /* semantic version number: https://semver.org */ -#define SH_RELEASE_DATE "2021-04-04" /* must be in this format for $((.sh.version)) */ +#define SH_RELEASE_DATE "2021-04-05" /* must be in this format for $((.sh.version)) */ #define SH_RELEASE_CPYR "(c) 2020-2021 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/array.c b/src/cmd/ksh93/sh/array.c index b0c00a856..59cea1abe 100644 --- a/src/cmd/ksh93/sh/array.c +++ b/src/cmd/ksh93/sh/array.c @@ -1522,6 +1522,7 @@ char *nv_endsubscript(Namval_t *np, register char *cp, int mode) } if(mode && np) { + Namarr_t *ap = nv_arrayptr(np); /* Block an attempt to alter a readonly array via subscript assignment or by appending the array. However need to allow instances of type variables. This exception is observed when np->nvflag has NV_BINARY and NV_LJUST set besides NV_RDONLY and NV_ARRAY. */ @@ -1530,9 +1531,6 @@ char *nv_endsubscript(Namval_t *np, register char *cp, int mode) errormsg(SH_DICT,ERROR_exit(1),e_readonly,nv_name(np)); UNREACHABLE(); } - - Namarr_t *ap = nv_arrayptr(np); - int scan = 0; #if SHOPT_FIXEDARRAY if((mode&NV_FARRAY) && !nv_isarray(np)) { @@ -1543,8 +1541,6 @@ char *nv_endsubscript(Namval_t *np, register char *cp, int mode) } } #endif /* SHOPT_FIXEDARRAY */ - if(ap) - scan = ap->nelem&ARRAY_SCAN; if((mode&NV_ASSIGN) && (cp[1]=='=' || cp[1]=='+')) mode |= NV_ADD; else if(ap && cp[1]=='.' && (mode&NV_FARRAY)) @@ -1555,8 +1551,6 @@ char *nv_endsubscript(Namval_t *np, register char *cp, int mode) else #endif /* SHOPT_FIXEDARRAY */ nv_putsub(np, sp, ((mode&NV_ADD)?ARRAY_ADD:0)|(cp[1]&&(mode&NV_ADD)?ARRAY_FILL:mode&ARRAY_FILL)); - if(scan) - ap->nelem |= scan; } if(quoted) stakseek(count); diff --git a/src/cmd/ksh93/sh/init.c b/src/cmd/ksh93/sh/init.c index feb80b01a..b8d2466d6 100644 --- a/src/cmd/ksh93/sh/init.c +++ b/src/cmd/ksh93/sh/init.c @@ -860,8 +860,6 @@ void sh_setmatch(Shell_t *shp,const char *v, int vsize, int nmatch, regoff_t mat } } -#define array_scan(np) ((nv_arrayptr(np)->nelem&ARRAY_SCAN)) - static char* get_match(register Namval_t* np, Namfun_t *fp) { struct match *mp = (struct match*)fp; diff --git a/src/cmd/ksh93/tests/arrays.sh b/src/cmd/ksh93/tests/arrays.sh index f94496b53..54dd1f5bf 100755 --- a/src/cmd/ksh93/tests/arrays.sh +++ b/src/cmd/ksh93/tests/arrays.sh @@ -709,5 +709,24 @@ actual="$(typeset -p foo)" # $expect and $actual are quoted intentionally [[ "$expect" == "$actual" ]] || err_exit "Multidimensional associative arrays are created with an extra array member (expected $expect, got $actual)" +# ====== +# Unsetting an array element removed all following elements if after subscript range expansion +# https://github.com/ksh93/ksh/issues/254 +for range in \ + 0..3 1..3 0..2 0..1 1..2 2..3 0..-1 0..-2 \ + .. 0.. 1.. 2.. 3.. ..0 ..1 ..2 ..3 \ + -4.. -3.. -2.. -1.. ..-4 ..-3 ..-2 ..-1 \ + -4..-1 -3..-1 -2..-1 -1..-1 +do + unset foo + set -A foo a b c d e f + eval "true \${foo[$range]}" + unset foo[2] # remove 3rd element 'c' + exp="a b d e f" + got=${foo[@]} + [[ $got == "$exp" ]] || err_exit "Expanding indexed array range \${foo[$range]} breaks 'unset foo[2]'" \ + "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +done + # ====== exit $((Errors<125?Errors:125))