From: Dan Williams <dan.j.williams@intel.com>
To: Li Zhijian <lizhijian@fujitsu.com>, <nvdimm@lists.linux.dev>
Cc: <linux-cxl@vger.kernel.org>, Li Zhijian <lizhijian@fujitsu.com>
Subject: RE: [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test
Date: Wed, 6 Dec 2023 13:31:54 -0800 [thread overview]
Message-ID: <6570e84ac75da_45e01294bd@dwillia2-xfh.jf.intel.com.notmuch> (raw)
In-Reply-To: <20231123023058.2963551-1-lizhijian@fujitsu.com>
I would summarize this as "Fix hex vs decimal confusion"
Li Zhijian wrote:
> size and resource are both decimal
Please always describe the "why" in the changelog including the
motivation for the change and the effect of not applying the patch. I.e.
how would someone know that they are hitting the problem that this patch
fixes. Sample output usually helps here.
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
> test/cxl-region-sysfs.sh | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
> index 8636392..ded7aa1 100644
> --- a/test/cxl-region-sysfs.sh
> +++ b/test/cxl-region-sysfs.sh
> @@ -123,6 +123,11 @@ readarray -t switch_decoders < <(echo $json | jq -r ".[].decoder")
> [ ${#switch_decoders[@]} -ne $nr_switch_decoders ] && err \
> "$LINENO: expected $nr_switch_decoders got ${#switch_decoders[@]} switch decoders"
>
> +decimal_to_hex()
> +{
> + printf "0x%x" $1
> +}
> +
> for i in ${switch_decoders[@]}
> do
> decoder=$(echo $json | jq -r ".[] | select(.decoder == \"$i\")")
> @@ -136,8 +141,8 @@ do
> [ $ig -ne $((r_ig << depth)) ] && err \
> "$LINENO: decoder: $i ig: $ig switch_ig: $((r_ig << depth))"
>
> - res=$(echo $decoder | jq -r ".resource")
> - sz=$(echo $decoder | jq -r ".size")
> + res=$(decimal_to_hex $(echo $decoder | jq -r ".resource"))
> + sz=$(decimal_to_hex $(echo $decoder | jq -r ".size"))
This feels like overkill, I think bash arithmentic operations can handle
hex conversion and decimal comparison...
> [ $sz -ne $region_size ] && err \
> "$LINENO: decoder: $i sz: $sz region_size: $region_size"
> [ $res -ne $region_base ] && err \
...i.e. does this solve the issue?
@@ -138,9 +138,9 @@ do
res=$(echo $decoder | jq -r ".resource")
sz=$(echo $decoder | jq -r ".size")
- [ $sz -ne $region_size ] && err \
+ (( sz != $region_size )) && err \
"$LINENO: decoder: $i sz: $sz region_size: $region_size"
- [ $res -ne $region_base ] && err \
+ (( $res != $region_base )) && err \
"$LINENO: decoder: $i base: $res region_base: $region_base"
done
prev parent reply other threads:[~2023-12-06 21:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-23 2:30 [ndctl PATCH 1/3] test/cxl-region-sysfs.sh: covert size and resource to hex before test Li Zhijian
2023-11-23 2:30 ` [ndctl PATCH 2/3] test/cxl-region-sysfs.sh: use operator '!=' to compare hexadecimal value Li Zhijian
2023-12-06 21:35 ` Dan Williams
2023-12-07 9:00 ` Zhijian Li (Fujitsu)
2023-11-23 2:30 ` [ndctl PATCH 3/3] test/cxl-region-sysfs.sh: Fix cxl-region-sysfs.sh: line 107: [: missing `]' Li Zhijian
2023-12-06 21:37 ` Dan Williams
2023-12-07 9:07 ` Zhijian Li (Fujitsu)
2023-12-06 21:31 ` Dan Williams [this message]
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=6570e84ac75da_45e01294bd@dwillia2-xfh.jf.intel.com.notmuch \
--to=dan.j.williams@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).