linux-kernel - Re: [PATCH] btrfs: remove unneeded variable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <69c7ac46-a469-d4d2-d4a7-c45f722816a0@gmx.com>
Date:   Fri, 17 Dec 2021 18:14:25 +0800
From:   Qu Wenruo <quwenruo.btrfs@....com>
To:     cgel.zte@...il.com, clm@...com
Cc:     josef@...icpanda.com, dsterba@...e.com,
        linux-btrfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Changcheng Deng <deng.changcheng@....com.cn>,
        Zeal Robot <zealci@....com.cn>
Subject: Re: [PATCH] btrfs: remove unneeded variable



On 2021/12/17 16:45, cgel.zte@...il.com wrote:
> From: Changcheng Deng <deng.changcheng@....com.cn>
>
> Remove unneeded variable used to store return value.
>
> Reported-by: Zeal Robot <zealci@....com.cn>
> Signed-off-by: Changcheng Deng <deng.changcheng@....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 = 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 = &trans->delayed_refs;
>
> @@ -4666,7 +4665,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>   	if (atomic_read(&delayed_refs->num_entries) == 0) {
>   		spin_unlock(&delayed_refs->lock);
>   		btrfs_debug(fs_info, "delayed_refs has NO entry");
> -		return ret;
> +		return 0;
>   	}
>
>   	while ((node = rb_first_cached(&delayed_refs->href_root)) != 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)