Re: [PATCH 29/29] btrfs: fixup_tree_root_location rename ret to ret2 and err to ret

Anand Jain
2024-04-16T10:45:07Z

On 4/16/24 18:42, Anand Jain wrote:
> On 3/20/24 02:24, Josef Bacik wrote:
>> On Tue, Mar 19, 2024 at 08:25:37PM +0530, Anand Jain wrote:
>>> Fix the code style for the return variable. First, rename ret to ret2,
>>> compile it, and then rename err to ret. This helps confirm that there
>>> are
>>> no instances of the old ret not renamed to ret2.
>>>
>>> Also, there is an opportunity to drop the initialization of ret to 0,
>>> with the first use of ret2 replaced with ret. However, due to the
>>> confusing
>>> git-diff, I refrain from doing that as of now.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>> ---
>>>   fs/btrfs/inode.c | 32 ++++++++++++++++----------------
>>>   1 file changed, 16 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
>>> index 952c92e6dfcf..d890cb5ab548 100644
>>> --- a/fs/btrfs/inode.c
>>> +++ b/fs/btrfs/inode.c
>>> @@ -5443,29 +5443,29 @@ static int fixup_tree_root_location(struct
>>> btrfs_fs_info *fs_info,
>>>       struct btrfs_root_ref *ref;
>>>       struct extent_buffer *leaf;
>>>       struct btrfs_key key;
>>> -    int ret;
>>> -    int err = 0;
>>> +    int ret2;
>>> +    int ret = 0;
>>>       struct fscrypt_name fname;
>>> -    ret = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name,
>>> 0, &fname);
>>> -    if (ret)
>>> -        return ret;
>>> +    ret2 = fscrypt_setup_filename(&dir->vfs_inode, &dentry->d_name,
>>> 0, &fname);
>>> +    if (ret2)
>>> +        return ret2;
>>>       path = btrfs_alloc_path();
>>>       if (!path) {
>>> -        err = -ENOMEM;
>>> +        ret = -ENOMEM;
>>>           goto out;
>>>       }
>>> -    err = -ENOENT;
>>> +    ret = -ENOENT;
>>>       key.objectid = dir->root->root_key.objectid;
>>>       key.type = BTRFS_ROOT_REF_KEY;
>>>       key.offset = location->objectid;
>>> -    ret = btrfs_search_slot(NULL, fs_info->tree_root, &key, path, 0,
>>> 0);
>>> -    if (ret) {
>>> -        if (ret < 0)
>>> -            err = ret;
>>> +    ret2 = btrfs_search_slot(NULL, fs_info->tree_root, &key, path,
>>> 0, 0);
>>> +    if (ret2) {
>>> +        if (ret2 < 0)
>>> +            ret = ret2;
>>>           goto out;
>>>       }
>>
>> This is another place where we can simply remove the ret2, just do
>>
>> ret = btrfs_search_slot();
>> if (ret) {
>>     if (ret > 0)
>>         ret = 0;
>
> Original code returned -ENOENT if btrfs_search_slot() return is > 0.
> I will retain it (instead of 0 as you suggested), I think it is correct.
>
> Thanks, Anand
>
>>     goto out;
>> }
>>
>>> @@ -5475,16 +5475,16 @@ static int fixup_tree_root_location(struct
>>> btrfs_fs_info *fs_info,
>>>           btrfs_root_ref_name_len(leaf, ref) != fname.disk_name.len)
>>>           goto out;
>>> -    ret = memcmp_extent_buffer(leaf, fname.disk_name.name,
>>> +    ret2 = memcmp_extent_buffer(leaf, fname.disk_name.name,
>>>                      (unsigned long)(ref + 1), fname.disk_name.len);
>>> -    if (ret)
>>> +    if (ret2)
>>>           goto out;
>>
>> And here simply do
>>
>> if (ret) {
>>     ret = 0;
>>     goto out;
>> }
>>

And here as well, if the name doesn't match, we returned -ENOENT
in the original code.

Thanks, Anand

>> Thanks,
>>
>> Josef
>