On Mon, Nov 01, 2021 at 04:15:15PM +0200, Nikolay Borisov wrote: > > > On 1.11.21 г. 16:26, Zorro Lang wrote: > > On Mon, Nov 01, 2021 at 03:56:58PM +0200, Nikolay Borisov wrote: > >> Current check is buggy because it can never trigger as even if > >> SCRATCH_DEV_POOL is not defined config_ndevs will get a value of 0 from > >> 'wc -w', this in turn makes 'typeset -p config_ndevs' always return 0, > >> triggering the existing check a noop. > >> > >> Fix this by explicitly checking for the presence of SCHRATC_DEV_POOL > >> > >> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx> > >> --- > >> > >> Eryu, > >> > >> Please use this patch as it's a more proper fix > >> > >> common/rc | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/common/rc b/common/rc > >> index 7f693d3922e8..07b69880eea6 100644 > >> --- a/common/rc > >> +++ b/common/rc > >> @@ -917,15 +917,15 @@ _scratch_dev_pool_get() > >> _fail "Usage: _scratch_dev_pool_get ndevs" > >> fi > >> > >> - local test_ndevs=$1 > >> - local config_ndevs=`echo $SCRATCH_DEV_POOL| wc -w` > >> - local -a devs="( $SCRATCH_DEV_POOL )" > >> - > >> - typeset -p config_ndevs >/dev/null 2>&1 > >> + typeset -p SCRATCH_DEV_POOL >/dev/null 2>&1 > >> if [ $? -ne 0 ]; then > >> _fail "Bug: cant find SCRATCH_DEV_POOL ndevs" > >> fi > >> > >> + local test_ndevs=$1 > >> + local config_ndevs=`echo $SCRATCH_DEV_POOL| wc -w` > >> + local -a devs="( $SCRATCH_DEV_POOL )" > > > > Yes, the logic of: > > "local config_ndevs=`echo $SCRATCH_DEV_POOL| wc -w` > > typeset -p config_ndevs >/dev/null 2>&1" > > looks weird. > > I can only speculate the idea was that `echo $SCRATCH_DEV_POOL| wc -w` > would fail to declare config_ndevs hence typeset -p config_ndevs would > return != 0, however that's not what is happening. Yes, "typeset -p $var" can help to check if a variable is declared, the original logic "typeset -p config_ndevs" always return 0. I think they might want to check if [ $config_ndevs -eq 0 ], or check "typeset -p SCRATCH_DEV_POOL" as you did above. Thanks, Zorro > > > > > Checking SCRATCH_DEV_POOL makes sense to me. > > > > Reviewed-by: Zorro Lang <zlang@xxxxxxxxxx> > > > > > >> + > >> if [ $config_ndevs -lt $test_ndevs ]; then > >> _notrun "Need at least test requested number of ndevs $test_ndevs" > >> fi > >> -- > >> 2.17.1 > >> > > >
