Re: [PATCH 2/6 v3] common/rc: _destroy_loop_device confirm arg1 is set

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



On Mon, Oct 30, 2023 at 2:15 PM Anand Jain <anand.jain@xxxxxxxxxx> wrote:
>
> Check if the dev arg1 is set before calling losetup -d on it.

Why?

Do we have any callers that call the function without an argument?
More comments below.

>
> Signed-off-by: Anand Jain <anand.jain@xxxxxxxxxx>
> ---
>  common/rc | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/common/rc b/common/rc
> index 18d2ddcf8e35..e7d6801b20e8 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -4150,7 +4150,10 @@ _create_loop_device()
>  _destroy_loop_device()
>  {
>         local dev=$1
> -       losetup -d $dev || _fail "Cannot destroy loop device $dev"
> +
> +       if [ ! -z $dev ]; then
> +               losetup -d $dev || _fail "Cannot destroy loop device $dev"
> +       fi

So this is just ignoring if no argument is given and the function does nothing.
This is quite the opposite of everywhere else, where we error out if a
necessary argument is missing.

btrfs/219 never calls this function without the argument, both before
and after this patchset, so I don't see why this patch is needed.
If we have any callers not passing the argument, I'd rather fix them
and make the function error out if no argument is given.

Thanks.

>  }
>
>  _scale_fsstress_args()
> --
> 2.39.3
>





[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux