Message ID | a5b8d4ef4c88f3c6bb87c81e58a75e91af1053c8.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:15PM +0530, Anand Jain wrote: > Variable err is the actual return value of this function, and variable ret > is a helper variable for err. So, rename them to ret and ret2 respectively. > This is just overkill, we don't ever need to hold both ret and ret2 in consistent state, you can just to a straight conversion of err to ret and it'll be fine. Thanks, Josef
在 2024/3/20 01:25, Anand Jain 写道: > Variable err is the actual return value of this function, and variable ret > is a helper variable for err. So, rename them to ret and ret2 respectively. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Agree with Josef, I didn't see any special reason why we want to hold both @ret and @err. We directly assign @err after btrfs_get_fs_root() already, and only continue if that @err is 0, so no need to hold two different values. And again, I don't like the @ret2 naming anyway. If we really need to hold two return values, I'd prefer to extract a function to do own init/cleanup work inside, so we only need to hold one @ret at higher level. Thanks, Qu > --- > fs/btrfs/root-tree.c | 36 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c > index 4bb538a372ce..869b50f05f39 100644 > --- a/fs/btrfs/root-tree.c > +++ b/fs/btrfs/root-tree.c > @@ -221,8 +221,8 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) > struct btrfs_path *path; > struct btrfs_key key; > struct btrfs_root *root; > - int err = 0; > - int ret; > + int ret = 0; > + int ret2; > > path = btrfs_alloc_path(); > if (!path) > @@ -235,18 +235,18 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) > while (1) { > u64 root_objectid; > > - ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0); > - if (ret < 0) { > - err = ret; > + ret2 = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0); > + if (ret2 < 0) { > + ret = ret2; > break; > } > > leaf = path->nodes[0]; > if (path->slots[0] >= btrfs_header_nritems(leaf)) { > - ret = btrfs_next_leaf(tree_root, path); > - if (ret < 0) > - err = ret; > - if (ret != 0) > + ret2 = btrfs_next_leaf(tree_root, path); > + if (ret2 < 0) > + ret = ret2; > + if (ret2 != 0) > break; > leaf = path->nodes[0]; > } > @@ -262,26 +262,26 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) > key.offset++; > > root = btrfs_get_fs_root(fs_info, root_objectid, false); > - err = PTR_ERR_OR_ZERO(root); > - if (err && err != -ENOENT) { > + ret = PTR_ERR_OR_ZERO(root); > + if (ret && ret != -ENOENT) { > break; > - } else if (err == -ENOENT) { > + } else if (ret == -ENOENT) { > struct btrfs_trans_handle *trans; > > btrfs_release_path(path); > > trans = btrfs_join_transaction(tree_root); > if (IS_ERR(trans)) { > - err = PTR_ERR(trans); > - btrfs_handle_fs_error(fs_info, err, > + ret = PTR_ERR(trans); > + btrfs_handle_fs_error(fs_info, ret, > "Failed to start trans to delete orphan item"); > break; > } > - err = btrfs_del_orphan_item(trans, tree_root, > + ret = btrfs_del_orphan_item(trans, tree_root, > root_objectid); > btrfs_end_transaction(trans); > - if (err) { > - btrfs_handle_fs_error(fs_info, err, > + if (ret) { > + btrfs_handle_fs_error(fs_info, ret, > "Failed to delete root orphan item"); > break; > } > @@ -312,7 +312,7 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) > } > > btrfs_free_path(path); > - return err; > + return ret; > } > > /* drop the root item for 'key' from the tree root */
diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c index 4bb538a372ce..869b50f05f39 100644 --- a/fs/btrfs/root-tree.c +++ b/fs/btrfs/root-tree.c @@ -221,8 +221,8 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) struct btrfs_path *path; struct btrfs_key key; struct btrfs_root *root; - int err = 0; - int ret; + int ret = 0; + int ret2; path = btrfs_alloc_path(); if (!path) @@ -235,18 +235,18 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) while (1) { u64 root_objectid; - ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0); - if (ret < 0) { - err = ret; + ret2 = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0); + if (ret2 < 0) { + ret = ret2; break; } leaf = path->nodes[0]; if (path->slots[0] >= btrfs_header_nritems(leaf)) { - ret = btrfs_next_leaf(tree_root, path); - if (ret < 0) - err = ret; - if (ret != 0) + ret2 = btrfs_next_leaf(tree_root, path); + if (ret2 < 0) + ret = ret2; + if (ret2 != 0) break; leaf = path->nodes[0]; } @@ -262,26 +262,26 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) key.offset++; root = btrfs_get_fs_root(fs_info, root_objectid, false); - err = PTR_ERR_OR_ZERO(root); - if (err && err != -ENOENT) { + ret = PTR_ERR_OR_ZERO(root); + if (ret && ret != -ENOENT) { break; - } else if (err == -ENOENT) { + } else if (ret == -ENOENT) { struct btrfs_trans_handle *trans; btrfs_release_path(path); trans = btrfs_join_transaction(tree_root); if (IS_ERR(trans)) { - err = PTR_ERR(trans); - btrfs_handle_fs_error(fs_info, err, + ret = PTR_ERR(trans); + btrfs_handle_fs_error(fs_info, ret, "Failed to start trans to delete orphan item"); break; } - err = btrfs_del_orphan_item(trans, tree_root, + ret = btrfs_del_orphan_item(trans, tree_root, root_objectid); btrfs_end_transaction(trans); - if (err) { - btrfs_handle_fs_error(fs_info, err, + if (ret) { + btrfs_handle_fs_error(fs_info, ret, "Failed to delete root orphan item"); break; } @@ -312,7 +312,7 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info) } btrfs_free_path(path); - return err; + return ret; } /* drop the root item for 'key' from the tree root */
Variable err is the actual return value of this function, and variable ret is a helper variable for err. So, rename them to ret and ret2 respectively. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/root-tree.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-)