Re: [PATCH 02/12] assign SCRATCH_DEV_POOL to an array

From: Filipe Manana <fdmanana@kernel.org>
To: Anand Jain <anand.jain@oracle.com>
Cc: fstests@vger.kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 02/12] assign SCRATCH_DEV_POOL to an array
Date: Thu, 15 Feb 2024 11:55:20 +0000	[thread overview]
Message-ID: <CAL3q7H6iCvMTu65Ngb3c+fNWMYoRVZRTqRcXJ4s0rM0HS66M1w@mail.gmail.com> (raw)
In-Reply-To: <366147ad0c29a6e4d4e0faa60231e66b81c7d678.1707969354.git.anand.jain@oracle.com>

On Thu, Feb 15, 2024 at 6:34 AM Anand Jain <anand.jain@oracle.com> wrote:
>
> Many test cases uses local variable to manage the names of each devices in

uses -> use
variable -> variables
devices -> device

> SCRATCH_DEV_POOL. Let _scratch_dev_pool_get set an array for it.
>
> Usage:
>
>         _scratch_dev_pool_get <n>
>
>         # device names are in the array SCRATCH_DEV_NAME.
>         ${SCRATCH_DEV_NAME[0]} ${SCRATCH_DEV_NAME[1]} ...
>
>         _scratch_dev_pool_put
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  common/rc | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/common/rc b/common/rc
> index 524ffa02aa6a..5e4afb2cd484 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -830,6 +830,8 @@ _spare_dev_put()
>  # required number of scratch devices by a-test-case excluding
>  # the replace-target and spare device. So this function will
>  # set SCRATCH_DEV_POOL to the specified number of devices.
> +# Also, this functions assigns array SCRATCH_DEV_NAME to the
> +# array SCRATCH_DEV_POOL.
>  #
>  # Usage:
>  #  _scratch_dev_pool_get() <ndevs>
> @@ -860,19 +862,28 @@ _scratch_dev_pool_get()
>         export SCRATCH_DEV_POOL_SAVED
>         SCRATCH_DEV_POOL=${devs[@]:0:$test_ndevs}
>         export SCRATCH_DEV_POOL
> +       SCRATCH_DEV_NAME=( $SCRATCH_DEV_POOL )
> +       export SCRATCH_DEV_NAME
>  }
>
>  _scratch_dev_pool_put()
>  {
> +       local ret1
> +       local ret2
> +
>         typeset -p SCRATCH_DEV_POOL_SAVED >/dev/null 2>&1
> -       if [ $? -ne 0 ]; then
> +       ret1=$?
> +       typeset -p SCRATCH_DEV_NAME >/dev/null 2>&1
> +       ret2=$?
> +       if [[ $ret1 -ne 0 || $ret2 -ne 0 ]]; then
>                 _fail "Bug: unset val, must call _scratch_dev_pool_get before _scratch_dev_pool_put"
>         fi
>
> -       if [ -z "$SCRATCH_DEV_POOL_SAVED" ]; then
> +       if [[ -z "$SCRATCH_DEV_POOL_SAVED" || -z SCRATCH_DEV_NAME ]]; then

missing dollar before SCRATCH_DEV_NAME... and should all be inside
double quotes, as -z is meant to test strings.
And as it's supposed to be an array, test it like:

-z "${SCRATCH_DEV_NAME[@]}"

>                 _fail "Bug: str empty, must call _scratch_dev_pool_get before _scratch_dev_pool_put"
>         fi
>
> +       export SCRATCH_DEV_NAME=""

Would rather assign an empty array (), as the variable is set to an
array in the get function.

Thanks.

>         export SCRATCH_DEV_POOL=$SCRATCH_DEV_POOL_SAVED
>         export SCRATCH_DEV_POOL_SAVED=""
>  }
> --
> 2.39.3
>
>