Re: [PATCH 03/29] btrfs: send_extent_data rename err to ret

Filipe Manana
2024-03-20T12:59:56Z

On Tue, Mar 19, 2024 at 2:56 PM Anand Jain <anand.jain@oracle.com> wrote:
>
> Simple renaming of the local variable err to ret.
>
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> fs/btrfs/send.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 50b4a76ac88e..7b67c884b8e1 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -5748,10 +5748,10 @@ static int send_extent_data(struct send_ctx *sctx, struct btrfs_path *path,
>
> sctx->cur_inode = btrfs_iget(root->fs_info->sb, sctx->cur_ino, root);
> if (IS_ERR(sctx->cur_inode)) {
> - int err = PTR_ERR(sctx->cur_inode);
> + int ret = PTR_ERR(sctx->cur_inode);
>
> sctx->cur_inode = NULL;
> - return err;
> + return ret;

I think you're taking the consensus of having a return value variable
named 'ret' to the extreme.
That's generally what we should do, especially if the variable is
declared in a function's top level scope, there are many variables and
it's not clear what's used for the return value.

But here?
It's a very short scope for an error path and it's clear enough we are
returning an error and there's no room for confusion here with other
variables.

Doing this type of change causes unnecessary churn and extra work for
future backports of bug fixes for example.

Thanks.

> }
> memset(&sctx->ra, 0, sizeof(struct file_ra_state));
> file_ra_state_init(&sctx->ra, sctx->cur_inode->i_mapping);
> --
> 2.38.1
>
>