Message-ID: <69c7ac46-a469-d4d2-d4a7-c45f722816a0@gmx.com>
Date:   Fri, 17 Dec 2021 18:14:25 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.4.0
Content-Language: en-US
To:     cgel.zte@gmail.com, clm@fb.com
Cc:     josef@toxicpanda.com, dsterba@suse.com,
        linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        Changcheng Deng <deng.changcheng@zte.com.cn>,
        Zeal Robot <zealci@zte.com.cn>
References: <20211217084522.452493-1-deng.changcheng@zte.com.cn>
From:   Qu Wenruo <quwenruo.btrfs@gmx.com>
Subject: Re: [PATCH] btrfs: remove unneeded variable
In-Reply-To: <20211217084522.452493-1-deng.changcheng@zte.com.cn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: quoted-printable
Precedence: bulk


On 2021/12/17 16:45, cgel.zte@gmail.com wrote:
> From: Changcheng Deng <deng.changcheng@zte.com.cn>
>
> Remove unneeded variable used to store return value.
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Changcheng Deng <deng.changcheng@zte.com.cn>
> ---
>   fs/btrfs/disk-io.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index d94a1ca856aa..d33575e56da2 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -4658,7 +4658,6 @@ static int btrfs_destroy_delayed_refs(struct btrfs=
_transaction *trans,
>   	struct rb_node *node;
>   	struct btrfs_delayed_ref_root *delayed_refs;
>   	struct btrfs_delayed_ref_node *ref;
> -	int ret =3D 0;

If you're removing @ret, it's better to also change the return value to
void.

Normally I would suggest you to checker the caller and return proper
error number instead.

But in this particular case, this function is only called in transaction
cleanup code, which means we have already aborted a transaction, thus
there is not much meaning to further error out, and the cleanup is OK.


So it would be fine for you to delete @ret, change the function to
return void.
And even better, to remove the "btrfs_" prefix of the function, as the
function is not really exported.


And a final suggest for your future patches, there is no need to bother
btrfs maintainers at all, just sending the mail to btrfs mailing list is
enough.

Thanks,
Qu
>
>   	delayed_refs =3D &trans->delayed_refs;
>
> @@ -4666,7 +4665,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs=
_transaction *trans,
>   	if (atomic_read(&delayed_refs->num_entries) =3D=3D 0) {
>   		spin_unlock(&delayed_refs->lock);
>   		btrfs_debug(fs_info, "delayed_refs has NO entry");
> -		return ret;
> +		return 0;
>   	}
>
>   	while ((node =3D rb_first_cached(&delayed_refs->href_root)) !=3D NULL=
) {
> @@ -4729,7 +4728,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs=
_transaction *trans,
>
>   	spin_unlock(&delayed_refs->lock);
>
> -	return ret;
> +	return 0;
>   }
>
>   static void btrfs_destroy_delalloc_inodes(struct btrfs_root *root)