| Message ID | b9fb4121b33c2b06ee0bcee74472c117481d2555.1710857863.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trivial adjustments for return variable coding style | expand |
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; 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
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
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 >
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; } @@ -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; btrfs_release_path(path); new_root = btrfs_get_fs_root(fs_info, location->objectid, true); if (IS_ERR(new_root)) { - err = PTR_ERR(new_root); + ret = PTR_ERR(new_root); goto out; } @@ -5492,11 +5492,11 @@ static int fixup_tree_root_location(struct btrfs_fs_info *fs_info, location->objectid = btrfs_root_dirid(&new_root->root_item); location->type = BTRFS_INODE_ITEM_KEY; location->offset = 0; - err = 0; + ret = 0; out: btrfs_free_path(path); fscrypt_free_filename(&fname); - return err; + return ret; } static void inode_tree_add(struct btrfs_inode *inode)
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(-)