Linux CXL
 help / color / mirror / Atom feed
From: "Verma, Vishal L" <vishal.l.verma@intel.com>
To: "lizhijian@fujitsu.com" <lizhijian@fujitsu.com>,
	"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Cc: "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>
Subject: Re: [ndctl PATCH v2] test/monitor.sh: Fix 2 bash syntax errors
Date: Wed, 16 Oct 2024 22:45:48 +0000	[thread overview]
Message-ID: <92fec0671d491ce6eecc075233cc0e09ddea52e8.camel@intel.com> (raw)
In-Reply-To: <20241016052042.1138320-1-lizhijian@fujitsu.com>

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>

>  		i=$((i + 1))
>  	done
>  	start_monitor "-r $monitor_region"
> @@ -146,6 +146,7 @@ test_filter_dimmevent()
>  	stop_monitor
>  
>  	inject_value=$($NDCTL list -H -d $monitor_dimms | jq -r .[]."health"."temperature_threshold")
> +	inject_value=${inject_value%.*}
>  	inject_value=$((inject_value + 1))
>  	start_monitor "-d $monitor_dimms -D dimm-media-temperature"
>  	inject_smart "-m $inject_value"


  reply	other threads:[~2024-10-16 22:45 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-16  5:20 [ndctl PATCH v2] test/monitor.sh: Fix 2 bash syntax errors Li Zhijian
2024-10-16 22:45 ` Verma, Vishal L [this message]
2024-10-17 15:57   ` Alison Schofield
2024-10-17  1:00 ` Alison Schofield
2024-10-17  3:30   ` Zhijian Li (Fujitsu)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=92fec0671d491ce6eecc075233cc0e09ddea52e8.camel@intel.com \
    --to=vishal.l.verma@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=lizhijian@fujitsu.com \
    --cc=nvdimm@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox