Re: [PATCH v3 6/6] btrfs: rename and optimize return variable in btrfs_find_orphan_roots

[David Sterba <dsterba@suse.cz>]

On Thu, May 16, 2024 at 07:12:15PM +0800, Anand Jain wrote:
> The variable err is the actual return value of this function, and the
> variable ret is a helper variable for err, which actually is not
> needed and can be handled just by err, which is renamed to ret.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v3: drop ret2 as there is no need for it.
> v2: n/a
>  fs/btrfs/root-tree.c | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/btrfs/root-tree.c b/fs/btrfs/root-tree.c
> index 33962671a96c..c11b0bccf513 100644
> --- a/fs/btrfs/root-tree.c
> +++ b/fs/btrfs/root-tree.c
> @@ -220,8 +220,7 @@ 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;
>  
>  	path = btrfs_alloc_path();
>  	if (!path)
> @@ -235,18 +234,19 @@ int btrfs_find_orphan_roots(struct btrfs_fs_info *fs_info)
>  		u64 root_objectid;
>  
>  		ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> -		if (ret < 0) {
> -			err = ret;
> +		if (ret < 0)
>  			break;
> -		}
> +		ret = 0;

Should this be handled when ret > 0? This would be unexpected and
probably means a corruption but simply overwriting the value does not
seem right.

>  
>  		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)
>  				break;
> +			if (ret > 0) {
> +				ret = 0;
> +				break;
> +			}
>  			leaf = path->nodes[0];
>  		}
>  
> @@ -261,26 +261,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;
>  			}
> @@ -311,7 +311,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 */
> -- 
> 2.38.1
>