1
0
Fork 0
mirror of git://git.code.sf.net/p/cdesktopenv/code synced 2025-03-09 15:50:02 +00:00

Fix += operator regressions (re: fae8862c) (#270)

The bugfix for BUG_CMDSPASGN backported in commit fae8862c caused
two regressions with the += operator:

1. The += operator did not append to variables. Reproducer:
     $ integer foo=3
     $ foo+=2 command eval 'echo $foo'
     2

2. The += operator ignored the readonly attribute, modifying readonly
   variables in the same manner as above. Reproducer
     $ readonly bar=str
     $ bar+=ing command eval 'echo $bar'
     ing

Both of the regressions above were caused by nv_putval() failing to
clone the variable from the previous scope into the invocation-local
scope. As a result, 'foo+=2' was effectively 0 + 2 (since ksh didn't
clone 3). The first regression was noticed during the development of
ksh93v-, so to fix both bugs I've backported the bugfix for the
regression from the ksh93v- 2013-10-10 alpha version:
https://www.mail-archive.com/ast-users@lists.research.att.com/msg00369.html

src/cmd/ksh93/sh/name.c:
- To fix both of the bugs above, find the variable to modify with
  nv_search(), then clone it into the invocation local scope. To
  fix the readonly bug as well, this is done before the NV_RDONLY
  check (otherwise np will be missing that attribute and be
  incorrectly modified in the invocation-local scope).
- Update a nearby comment describing what sh_assignok() does (per this
  comment: https://github.com/ksh93/ksh/pull/249#issuecomment-811381759)

src/cmd/ksh93/tests/builtins.sh:
- Add regression tests for both of the now fixed regressions,
  loosely based on the regression tests in ksh93v-.
This commit is contained in:
Johnothan King 2021-04-11 17:24:33 -07:00 committed by GitHub
parent 6629c22308
commit 75796a9c75
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 53 additions and 7 deletions

View file

@ -1601,15 +1601,23 @@ void nv_putval(register Namval_t *np, const char *string, int flags)
#if SHOPT_FIXEDARRAY
Namarr_t *ap;
#endif /* SHOPT_FIXEDARRAY */
if(!(flags&NV_RDONLY) && nv_isattr (np, NV_RDONLY))
/*
* When we are in an invocation-local scope and we are using the
* += operator, clone the variable from the previous scope.
*/
if((flags&NV_APPEND) && nv_isnull(np) && shp->var_tree->view)
{
Namval_t *mp = nv_search(np->nvname,shp->var_tree->view,0);
if(mp)
nv_clone(mp,np,0);
}
/* Don't alter readonly variables */
if(!(flags&NV_RDONLY) && nv_isattr(np,NV_RDONLY))
{
errormsg(SH_DICT,ERROR_exit(1),e_readonly, nv_name(np));
UNREACHABLE();
}
/*
* The following could cause the shell to fork if assignment
* would cause a side effect
*/
/* Create a local scope when inside of a virtual subshell */
shp->argaddr = 0;
if(shp->subshell && !nv_local && !(flags&NV_RDONLY))
np = sh_assignok(np,1);