Skip to content

Commit

Permalink
Fix test -v for numeric types & set/unset state for short int
Browse files Browse the repository at this point in the history
This commit fixes two interrelated problems.

1. The -v unary test/[/[[ operator is documented to test if a
   variable is set. However, it always returns true for variable
   names with a numeric attribute, even if the variable has not
   been given a value. Reproducer:
	$ ksh -o nounset -c 'typeset -i n; [[ -v n ]] && echo $n'
	ksh: n: parameter not set
   That is clearly wrong; 'echo $n' should never be reached and the
   error should not occur, and does not occur on mksh or bash.

2. Fixing the previous problem revealed serious breakage in short
   integer type variables that was being masked. After applying
   that fix and then executing 'typeset -si var=0':
   - The conditional assignment expansions ${var=123} and
     ${var:=123} assigned 123 to var, even though it was set to 0.
   - The expansions ${var+s} and ${var:+n} incorrectly acted as if
     the variable was unset and empty, respectively.
   - '[[ -v var ]]' and 'test -v var' incorrectly returned false.
   The problems were caused by a different storage method for short
   ints. Their values were stored directly in the 'union Value'
   member of the Namval_t struct, instead of allocated on the stack
   and referred to by a pointer, as regular integers and all other
   types do. This inherently broke nv_isnull() as this leaves no
   way to distinguish between a zero value and no value at all.
   (I'm also pretty sure it's undefined behaviour in C to check for
   a null pointer at the address where a short int is stored.)
   The fix is to store short ints like other variables and refer
   to them by pointers. The NV_INT16P combined bit mask already
   existed for this, but nv_putval() did not yet support it.

src/cmd/ksh93/bltins/test.c: test_unop():
- Fix problem 1. For -v, only check nv_isnull() and do not check
  for the NV_INTEGER attribute (which, by the way, is also used
  for float variables by combining it with other bits).
  See also 5aba0c7 where we recently fixed nv_isnull() to
  work properly for all variable types including short ints.

src/cmd/ksh93/sh/name.c: nv_putval():
- Fix problem 2, part 1. Add support for NV_INT16P. The code is
  simply copied and adapted from the code for regular integers, a
  few lines further on. The regular NV_SHORT code is kept as this
  is still used for some special variables like ${.sh.level}.

src/cmd/ksh93/bltins/typeset.c: b_typeset():
- Fix problem 2, part 2. Use NV_INT16P instead of NV_SHORT.

src/cmd/ksh93/tests/attributes.sh:
- Add set/unset/empty/nonempty tests for all numeric types.

src/cmd/ksh93/tests/bracket.sh,
src/cmd/ksh93/tests/comvar.sh:
- Update a couple of existing tests.
- Add test for [[ -v var ]] and [[ -n ${var+s} ]] on unset
  and empty variables with many attributes.

src/cmd/ksh93/COMPATIBILITY:
- Add a note detailing the change to test -v.

src/cmd/ksh93/data/builtins.c,
src/cmd/ksh93/sh.1:
- Correct 'typeset -C' documentation. Variables declared as
  compound are *not* initially unset, but initially have the empty
  compound value. 'typeset' outputs them as:
	typeset -C foo=()
  and not:
	typeset -C foo
  and nv_isnull() is never true for them. This may or may not
  technically be a bug. I don't think it's worth changing, but
  it should at least be documented correctly.
  • Loading branch information
McDutchie committed Mar 10, 2021
1 parent 4a8072e commit d4adc8f
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 11 deletions.
5 changes: 5 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ Any uppercase BUG_* names are modernish shell bug IDs.
- The ${!foo@} and ${!foo*} expansions yield variable names beginning with foo,
but excluded 'foo' itself. The fix for this is now backported from 93v- beta.

- test -v var, [ -v var ], and [[ -v var ]] did not correctly test if a
variable is set or unset after it has been given a numeric attribute with
'typeset' but not yet assigned a value. This has been fixed so that
[[ -v var ]] is now equivalent to [[ -n ${var+set} ]] as documented.

2021-03-07:

- Fixed the typeset -p display of short integers without an assigned value.
Expand Down
5 changes: 5 additions & 0 deletions src/cmd/ksh93/COMPATIBILITY
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,11 @@ For more details, see the NEWS file and for complete details, see the git log.
18. The variable name search expansions ${!prefix@} and ${!prefix*} now
also include the variable 'prefix' itself in the possible results.

19. [[ -v var ]] is now properly equivalent to [[ -n ${var+set} ]].
Undocumented special-casing for numeric types has been removed.
For example, the following no longer produces an unexpected error:
$ ksh -o nounset -c 'float n; [[ -v n ]] && echo $n'

____________________________________________________________________________

KSH-93 VS. KSH-88
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/bltins/test.c
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ int test_unop(Shell_t *shp,register int op,register const char *arg)
return(nv_arrayisset(np,ap));
if(*arg=='I' && strcmp(arg,"IFS")==0)
return(nv_getval(np)!=NULL); /* avoid BUG_IFSISSET */
return(!nv_isnull(np) || nv_isattr(np,NV_INTEGER));
return(!nv_isnull(np));
}
default:
{
Expand Down
7 changes: 4 additions & 3 deletions src/cmd/ksh93/bltins/typeset.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ int b_typeset(int argc,register char *argv[],Shbltin_t *context)
if(shortint)
{
flag &= ~NV_LONG;
flag |= NV_SHORT|NV_INTEGER;
flag |= NV_INT16P;
}
else
flag |= NV_INTEGER;
Expand All @@ -331,7 +331,8 @@ int b_typeset(int argc,register char *argv[],Shbltin_t *context)
if(shortint)
{
shortint = 0;
flag &= ~NV_SHORT;
/* Turn off the NV_INT16P bits except the NV_INTEGER bit */
flag &= ~(NV_INT16P & ~NV_INTEGER);
}
tdata.wctname = e_tolower;
flag |= NV_UTOL;
Expand All @@ -357,7 +358,7 @@ int b_typeset(int argc,register char *argv[],Shbltin_t *context)
if(flag&NV_INTEGER)
{
flag &= ~NV_LONG;
flag |= NV_SHORT;
flag |= NV_INT16P;
}
break;
case 't':
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/ksh93/data/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1838,7 +1838,7 @@ const char sh_opttypeset[] =
"become index \b0\b.]"
"[C?Compound variable. Each \aname\a will be a compound variable. If "
"\avalue\a names a compound variable it will be copied to \aname\a. "
"Otherwise if the variable already exists, it will first be unset.]"
"Otherwise the variable is assigned the empty compound value.]"
"[E]#?[n:=10?Floating point number represented in scientific notation. "
"\an\a specifies the number of significant figures when the "
"value is expanded.]"
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/sh.1
Original file line number Diff line number Diff line change
Expand Up @@ -7561,13 +7561,13 @@ Subscripts are strings rather than arithmetic
expressions.
.TP
.B \-C
causes each
Causes each
.I vname\^
to be a compound variable. If
.I value\^
names a compound variable, it is copied into
.IR vname .
Otherwise, it unsets each
Otherwise, the empty compound value is assigned to
.IR vname .
.TP
.B \-a
Expand Down
14 changes: 12 additions & 2 deletions src/cmd/ksh93/sh/name.c
Original file line number Diff line number Diff line change
Expand Up @@ -1721,7 +1721,7 @@ void nv_putval(register Namval_t *np, const char *string, int flags)
}
else
{
int32_t l=0,ol=0;
int32_t l=0;
if(flags&NV_INTEGER)
{
if((flags&NV_DOUBLE) == NV_DOUBLE)
Expand Down Expand Up @@ -1764,7 +1764,16 @@ void nv_putval(register Namval_t *np, const char *string, int flags)
}
if(nv_size(np) <= 1)
nv_setsize(np,10);
if(nv_isattr (np, NV_SHORT))
if(nv_isattr(np,NV_INT16P)==NV_INT16P)
{
int16_t os=0;
if(!up->sp)
up->sp = new_of(int16_t,0);
else if(flags&NV_APPEND)
os = *(up->sp);
*(up->sp) = os+(int16_t)l;
}
else if(nv_isattr(np,NV_SHORT))
{
int16_t s=0;
if(flags&NV_APPEND)
Expand All @@ -1774,6 +1783,7 @@ void nv_putval(register Namval_t *np, const char *string, int flags)
}
else
{
int32_t ol=0;
if(!up->lp)
up->lp = new_of(int32_t,0);
else if(flags&NV_APPEND)
Expand Down
28 changes: 28 additions & 0 deletions src/cmd/ksh93/tests/attributes.sh
Original file line number Diff line number Diff line change
Expand Up @@ -667,5 +667,33 @@ exp=$'00000\n000\n0000000'
[[ $(typeset -Z4 x; typeset -rZ x; typeset -p x) == 'typeset -r -Z 0 -R 0 x' ]] || err_exit "typeset -rZ failed to set new size."
[[ $(typeset -bZ4 x; typeset -rbZ x; typeset -p x) == 'typeset -r -b -Z 0 -R 0 x' ]] || err_exit "typeset -rbZ failed to set new size."

# ======
# set/unset tests for numeric types
for flag in i s si ui us usi uli E F X lX
do
unset var
typeset "-$flag" var
[[ -v var ]] && err_exit "[[ -v var ]] should return false after typeset -$flag var"
[[ -z ${var+s} ]] || err_exit "\${var+s} should be empty after typeset -$flag var (got '${var+s}')"
[[ -z ${var:+n} ]] || err_exit "\${var:+n} should be empty after typeset -$flag var (got '${var+n}')"
[[ ${var-u} == 'u' ]] || err_exit "\${var-u} should be 'u' after typeset -$flag var (got '${var-u}')"
[[ ${var:-e} == 'e' ]] || err_exit "\${var:-e} should be 'e' after typeset -$flag var (got '${var:-e}')"
(( ${var=1} == 1 )) || err_exit "\${var=1} should yield 1 after typeset -$flag var (got '$var')"
unset var
typeset "-$flag" var
(( ${var:=1} == 1 )) || err_exit "\${var:=1} should yield 1 after typeset -$flag var (got '$var')"
unset var
typeset "-$flag" var=0
[[ -v var ]] || err_exit "[[ -v var ]] should return true after typeset -$flag var=0"
[[ ${var+s} == 's' ]] || err_exit "\${var+s} should be 's' after typeset -$flag var=0"
[[ ${var:+n} == 'n' ]] || err_exit "\${var:+n} should be 'n' after typeset -$flag var=0"
[[ ${var-u} != 'u' ]] || err_exit "\${var-u} should be 0 after typeset -$flag var (got '${var-u}')"
[[ ${var:-e} != 'e' ]] || err_exit "\${var:-e} should be 0 after typeset -$flag var (got '${var:-e}')"
(( ${var=1} == 0 )) || err_exit "\${var=1} should yield 0 after typeset -$flag var=0 (got '$var')"
unset var
typeset "-$flag" var=0
(( ${var:=1} == 0 )) || err_exit "\${var:=1} should yield 0 after typeset -$flag var=0 (got '$var')"
done

# ======
exit $((Errors<125?Errors:125))
21 changes: 20 additions & 1 deletion src/cmd/ksh93/tests/bracket.sh
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,9 @@ $SHELL 2> /dev/null -c '[[ "]" == ~(E)[]] ]]' || err_exit 'pattern "~(E)[]]" doe
unset var
[[ -v var ]] && err_exit '[[ -v var ]] should be false after unset var'
float var
[[ -v var ]] || err_exit '[[ -v var ]] should be true after float var'
[[ -v var ]] && err_exit '[[ -v var ]] should be false after float var'
unset var; float var=
[[ -v var ]] || err_exit '[[ -v var ]] should be true after float var='
unset var
[[ -v var ]] && err_exit '[[ -v var ]] should be false after unset var again'

Expand Down Expand Up @@ -389,5 +391,22 @@ error=$(set +x; "$SHELL" -c '[[ AATAAT =~ (AAT){2} ]]' 2>&1) \
error=$(set +x; "$SHELL" -c '[[ AATAATCCCAATAAT =~ (AAT){2}CCC(AAT){2} ]]' 2>&1) \
|| err_exit "[[ AATAATCCCAATAAT =~ (AAT){2}CCC(AAT){2} ]] does not match${error:+ (got $(printf %q "$error"))}"

# ======
# The -v unary operator should work for names with all type attributes.
empty=
for flag in a b i l n s si u ui usi uli E F H L Mtolower Mtoupper R X lX S Z
do unset var
typeset "-$flag" var
[[ -v var ]] && err_exit "[[ -v var ]] should be false for unset var with attribute -$flag"
[[ -n ${var+s} ]] && err_exit "[[ -n \${var+s} ]] should be false for unset var with attribute -$flag"
unset var
case $flag in
n) typeset -n var=empty ;;
*) typeset "-$flag" var= ;;
esac
[[ -v var ]] || err_exit "[[ -v var ]] should be true for empty var with attribute -$flag"
[[ -n ${var+s} ]] || err_exit "[[ -n \${var+s} ]] should be true for empty var with attribute -$flag"
done

# ======
exit $((Errors<125?Errors:125))
2 changes: 1 addition & 1 deletion src/cmd/ksh93/tests/comvar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ expected=$'typeset -C -a mica01=([4]=(a_string=\'foo bar\';some_stuff=hello))'
[[ $(typeset -p mica01) == "$expected" ]] || err_exit 'appened to indexed array compound variable not working'

unset x
compound x=( integer x ; )
compound x=( integer x= ; )
[[ ! -v x.x ]] && err_exit 'x.x should be set'
expected=$'(\n\ttypeset -l -i x=0\n)'
[[ $(print -v x) == "$expected" ]] || err_exit "'print -v x' should be $expected"
Expand Down

0 comments on commit d4adc8f

Please sign in to comment.