[openstack-dev] [devstack] set -o nounset in devstack?

[Sean Dague]

I got bit by another bug yesterday where there was a typo between
variables in the source tree. So I started down the process of set -o
nounset to see how bad it would be to prevent that in the future.

There are 2 major classes of issues where the code is functioning fine,
but is caught by nounset:

FOO=$(trueorfalse True $FOO)

if [[ -n $FOO ]]; ...


The trueorfalse issue can be fixed if we change the function to be:

function trueorfalse {
local xtrace=$(set +o | grep xtrace)
set +o xtrace
local default=$1
local testval=${!2+x}

[[ -z $testval ]]  { echo $default; return; }
[[ 0 no No NO false False FALSE =~ $testval ]]  { echo
False; return; }
[[ 1 yes Yes YES true True TRUE =~ $testval ]]  { echo True;
return; }
echo $default
$xtrace
}


FOO=$(trueorfalse True FOO)

... then works.

the -z and -n bits can be addressed with either FOO=${FOO:-} or an isset
function that interpolates. FOO=${FOO:-} actually feels better to me
because it's part of the spirit of things.

I've found a few bugs already even though I'm probably only about 20% to
a complete run working.


So... the question is, is this worth it? It's going to have fall out in
lesser used parts of the code where we don't catch things (like -o
errexit did). However it should help flush out a class of bugs in the
process.

Opinions from devstack contributors / users welcomed.

-Sean

-- 
Sean Dague
http://dague.net

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Re: [openstack-dev] [devstack] set -o nounset in devstack?

[Dean Troyer]

On Fri, Dec 5, 2014 at 6:45 AM, Sean Dague s...@dague.net wrote:

 I got bit by another bug yesterday where there was a typo between
 variables in the source tree. So I started down the process of set -o
 nounset to see how bad it would be to prevent that in the future.


[...]


 The trueorfalse issue can be fixed if we change the function to be:

 function trueorfalse {
 local xtrace=$(set +o | grep xtrace)
 set +o xtrace
 local default=$1
 local testval=${!2+x}

 [[ -z $testval ]]  { echo $default; return; }


There should be an $xtrace in that return path


 [[ 0 no No NO false False FALSE =~ $testval ]]  { echo
 False; return; }
 [[ 1 yes Yes YES true True TRUE =~ $testval ]]  { echo True;
 return; }
 echo $default
 $xtrace
 }


 FOO=$(trueorfalse True FOO)

 ... then works.


I'm good with this.


 the -z and -n bits can be addressed with either FOO=${FOO:-} or an isset
 function that interpolates. FOO=${FOO:-} actually feels better to me
 because it's part of the spirit of things.


I think I agree, but we have a lot og is_*() functions so that wouldn't be
to far of a departure, I could be convinced either way I suppose.  This is
going to be the hard part of the cleanup and ongoing enforcement.


 So... the question is, is this worth it? It's going to have fall out in
 lesser used parts of the code where we don't catch things (like -o
 errexit did). However it should help flush out a class of bugs in the
 process.


This is going to be a long process to do the change, I think we will need
to bracket parts of the code as they get cleaned up to avoid regressions
slipping in.

dt

-- 

Dean Troyer
dtro...@gmail.com
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev