dermotbradley/create-alpine-disk-image

Parameter expansion problem in evaluation of variables

Closed this issue · 6 comments

I had a doubt on the way you use parameter expansion for checking variables and settings. Unless I am missing something here, you should probably be using if [ -n "{setting:+x}" ] instead of if [ -n "${setting+x}" ] which always evaluates to true.

See the following small script to check both cases with a few examples:

#! /bin/sh

FOO1="FOO"
FOO2='FOO'
FOO3=$(expr 0 + 1)
FOO4=""
FOO5=''
FOO6=

CNT=0

testcase () {
  CNT=$(expr ${CNT} + 1)
  echo "Example ${CNT}"
  echo -n "Case 1: "
  if [ -n "${1:+x}" ]; then echo "true"; else echo "false"; fi
  echo -n "Case 2: "
  if [ -n "${1+x}" ]; then echo "true"; else echo "false"; fi
  echo
}

testcase "${FOO1}"
testcase "${FOO2}"
testcase "${FOO3}"
testcase "${FOO4}"
testcase "${FOO5}"
testcase "${FOO6}"
testcase "${FOO7}"

Unless I am missing something here, you should probably be using if [ -n "{setting:+x}" ] instead of if [ -n "${setting+x}" ] which always evaluates to true.

[ -n "${setting+x}" ] does not always evaluate to true.

If you look at https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html#tag_02_06_02 at the table in section 2.6.2 Parameter Expansion, there are 3 situations to be considered:

  • setting is not set
  • setting is set to ""
  • setting is set to something other than ""

The behaviour of the 2 types of test differs only in the 2nd situation above, when setting is set to "" (what the table describes as "Set But Null").

It is possible there may be some instances in my script where a variable being checked via "${variable+x}" is actually any empty string, rather than not set at all - I will check the code and fix any such occurances I find.

Also I don't think your test script is doing what you think it is doing. I changed the line:

echo "Example ${CNT}"

to also show the string it is working on:

echo "Example ${CNT}, for '$1'"

which gives the following output:

Example 1, for 'FOO'
Case 1: true
Case 2: true

Example 2, for 'FOO'
Case 1: true
Case 2: true

Example 3, for '1'
Case 1: true
Case 2: true

Example 4, for ''
Case 1: false
Case 2: true

Example 5, for ''
Case 1: false
Case 2: true

Example 6, for ''
Case 1: false
Case 2: true

Example 7, for ''
Case 1: false
Case 2: true

You can see that only examples 1-3 are given any actual (non empty) string to work with and both cases behave the same. The other examples are given empty strings which is exactly where the test behaviour (with and without a ":") is expected to differ.

You can see that only examples 1-3 are given any actual (non empty) string to work with and both cases behave the same. The other examples are given empty strings which is exactly where the test behaviour (with and without a ":") is expected to differ.

That's exactly the point I was trying to make. If for example you test for an undeclared setting, wouldn't it evaluate to true if you don't use colon, thus making the script behave in unintended ways?

If for example you test for an undeclared setting, wouldn't it evaluate to true if you don't use colon, thus making the script behave in unintended ways?

Simple example:

if [ -n "${option+x}" ]; then
  echo "set"
else
  echo "not set"
fi

and

if [ -n "${option:+x}" ]; then
  echo "set "
else
  echo "not set"
fi

Both behave the same if in the line before them you put unset option - they output not set. They also both behave the same if instead you put option="abcdef" - they output set

The only time they behave differently is if you put option="".

I would prefer testing for declared and not null variable, but fair enough. Thanks for clarifying.

For boolean settings (i.e. "--disable-this", "--enable-that") I'm only setting the related variable if the script option is actually specified.

For other options that have actual multiple values then I do set the related variable to at least a default value if the option is not specified.

Makes sense.