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

Anand Jain
2024-04-16T10:42:39Z

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;
> }
>
> Thanks,
>
> Josef