Re: [ndctl PATCH v2] test/monitor.sh: Fix 2 bash syntax errors

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Oct 16, 2024 at 10:45:48PM +0000, Vishal Verma wrote:
> On Wed, 2024-10-16 at 13:20 +0800, Li Zhijian wrote:
> > $ grep -w line build/meson-logs/testlog.txt
> > test/monitor.sh: line 99: [: too many arguments
> > test/monitor.sh: line 99: [: nmem0: binary operator expected
> > test/monitor.sh: line 149: 40.0: syntax error: invalid arithmetic operator (error token is ".0")
> > 
> > - monitor_dimms could be a string with multiple *spaces*, like: "nmem0 nmem1 nmem2"
> > - inject_value is a float value, like 40.0, which need to be converted to
> >   integer before operation: $((inject_value + 1))
> > 
> > Some features have not been really verified due to these errors
> > 
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > ---
> > V1:
> >  V1 has a mistake which overts to integer too late.
> >  Move the conversion forward before the operation
> > ---
> >  test/monitor.sh | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/test/monitor.sh b/test/monitor.sh
> > index c5beb2c..7809a7c 100755
> > --- a/test/monitor.sh
> > +++ b/test/monitor.sh
> > @@ -96,7 +96,7 @@ test_filter_region()
> >  	while [ $i -lt $count ]; do
> >  		monitor_region=$($NDCTL list -R -b $smart_supported_bus | jq -r .[$i].dev)
> >  		monitor_dimms=$(get_monitor_dimm "-r $monitor_region")
> > -		[ ! -z $monitor_dimms ] && break
> > +		[ ! -z "$monitor_dimms" ] && break
> 
> [ ! -z "..." ] is a bit of a double negative, while we are changing
> this, I'd suggest cleaning up a bit more such as:
> 
>    if [[ "$monitor_dimms" ]]; then
>    	break
>    fi
> 
> Other than that looks good,
> 
> Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

BTW - I second Vishal's suggestion here.
Shellcheck is catching bad syntax but may not be suggesting the
best syntax as an alternative. 

--Alison

snip