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

Qu Wenruo
2024-03-19T21:10:20Z

在 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 */