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"
next prev parent 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