[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fstests
Subject:    Re: [PATCH 2/6 v3] common/rc: _destroy_loop_device confirm arg1 is set
From:       Anand Jain <anand.jain () oracle ! com>
Date:       2023-10-30 23:48:22
Message-ID: f3b1e097-b692-4016-b44f-fa79941709ae () oracle ! com
[Download RAW message or body]



On 10/31/23 00:21, Filipe Manana wrote:
> On Mon, Oct 30, 2023 at 2:15 PM Anand Jain <anand.jain@oracle.com> 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@oracle.com>
>> ---
>>   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.

You are right. We no longer need this patch. In version 2, we had the
last mount called with or without temp-fsid. That's being removed,
and initialization is taken care of, so we can drop this patch.
I'll drop this patch.

> 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, Anand

[prev in list] [next in list] [prev in thread] [next in thread]