Skip to content

Commit

Permalink
The unalias builtin should return an error for non-existent aliases
Browse files Browse the repository at this point in the history
This commit fixes a bug that caused unalias to return a zero status
when it tries to remove an alias twice. The following set of commands
will no longer end with an error:

$ alias foo=bar
$ unalias foo
$ unalias foo && echo 'Error'

This commit is based on the fix present in ksh2020, but it has been
extended with another bugfix. The initial fix for this problem tried to
remove aliases from the alias tree without accounting for NV_NOFREE. This
caused any attempt to remove a predefined aliases (e.g. `unalias float`)
to trigger an error with free, as all predefined aliases are in read-only
memory. The fix for this problem is to set NV_NOFREE when removing aliases
from the alias tree, but only if the alias is in read-only memory. All
other aliases must be freed from memory to prevent memory leaks.

I'll also note that I am using an `isalias` variable rather than the `type`
enum from ksh2020, as the `VARIABLE` value is never used and was replaced
with a bool called `aliases` in the ksh2020 release. The `isalias` variable
is an int as the ksh93u+ codebase does not use C99 bools.

Previous discussion: att#909

- src/cmd/ksh93/bltins/typeset.c:
  Remove aliases from the alias tree by using nv_delete. NV_NOFREE
  is only used when it is necessary.

- src/cmd/ksh93/tests/alias.sh:
  Add two regression tests for the bugs fixed by this commit.

(cherry picked from commit 16d5ea9b52ba51f9d1bca115ce8f4f18e97abbc4)
  • Loading branch information
JohnoKing authored and McDutchie committed Jun 11, 2020
1 parent 12e7005 commit f7251bc
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
5 changes: 5 additions & 0 deletions NEWS
Expand Up @@ -4,6 +4,11 @@ For full details, see the git log at:

Any uppercase BUG_* names are modernish shell bug IDs.

2020-06-09:

- The 'unalias' builtin will now return a non-zero status if it tries
to remove a previously set alias that is not currently set.

2020-06-08:

- Fix an issue with the up arrow key in Emacs editing mode.
Expand Down
14 changes: 12 additions & 2 deletions src/cmd/ksh93/bltins/typeset.c
Expand Up @@ -1161,11 +1161,13 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
register const char *name;
volatile int r;
Dt_t *dp;
int nflag=0,all=0,isfun,jmpval;
int nflag=0,all=0,isalias=0,isfun,jmpval;
struct checkpt buff;
NOT_USED(argc);
if(troot==shp->alias_tree)
if(troot==shp->alias_tree) {
isalias = 1;
name = sh_optunalias;
}
else
name = sh_optunset;
while(r = optget(argv,name)) switch(r)
Expand Down Expand Up @@ -1281,6 +1283,7 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
if(shp->subshell)
np=sh_assignok(np,0);
}

if(!nv_isnull(np) || nv_size(np) || nv_isattr(np,~(NV_MINIMAL|NV_NOFREE)))
_nv_unset(np,0);
if(troot==shp->var_tree && shp->st.real_fun && (dp=shp->var_tree->walk) && dp==shp->st.real_fun->sdict)
Expand All @@ -1298,6 +1301,13 @@ static int unall(int argc, char **argv, register Dt_t *troot, Shell_t* shp)
while((troottmp = troottmp->view) && (np = nv_search(name,troottmp,0)) && is_afunction(np))
nv_delete(np,troottmp,0);
}
/* The alias has been unset by call to _nv_unset, remove it from the tree */
else if(isalias) {
if(nv_isattr(np, NV_NOFREE))
nv_delete(np,troot,NV_NOFREE); /* The alias is in read-only memory (shtab_aliases) */
else
nv_delete(np,troot,0);
}
#if 0
/* causes unsetting local variable to expose global */
else if(shp->var_tree==troot && shp->var_tree!=shp->var_base && nv_search((char*)np,shp->var_tree,HASH_BUCKET|HASH_NOSCOPE))
Expand Down
12 changes: 11 additions & 1 deletion src/cmd/ksh93/tests/alias.sh
Expand Up @@ -103,6 +103,16 @@ fi
( alias :pr=print) 2> /dev/null || err_exit 'alias beginning with : fails'
( alias p:r=print) 2> /dev/null || err_exit 'alias with : in name fails'

unalias no_such_alias && err_exit 'unalias should return non-zero for unknown alias'
unalias no_such_alias && err_exit 'unalias should return non-zero for unknown alias'

# ======
# Attempting to unalias a previously set alias twice should be an error
alias foo=bar
unalias foo
unalias foo && err_exit 'unalias should return non-zero when a previously set alias is unaliased twice'

# Removing a predefined alias should work without an error from free(3)
$SHELL -c 'unalias history' 2> /dev/null || err_exit 'removing a predefined alias does not work'

# ======
exit $((Errors<125?Errors:125))

0 comments on commit f7251bc

Please sign in to comment.