linux-kernel - Re: [PATCH] btrfs: turn unpin_extent

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: <20230720110438.GU20457@twin.jikos.cz>
Date:   Thu, 20 Jul 2023 13:04:38 +0200
From:   David Sterba <dsterba@...e.cz>
To:     Luís Henriques <lhenriques@...e.de>
Cc:     Chris Mason <clm@...com>, Josef Bacik <josef@...icpanda.com>,
        David Sterba <dsterba@...e.com>, linux-btrfs@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] btrfs: turn unpin_extent_cache() into a void function

On Tue, Jul 18, 2023 at 06:39:06PM +0100, Luís Henriques wrote:
> The value of the 'ret' variable is never changed in function
> unpin_extent_cache().  And since the only caller of this function doesn't
> check the return value, it can simply be turned into a void function.
> 
> Signed-off-by: Luís Henriques <lhenriques@...e.de>
> ---
>  fs/btrfs/extent_map.c | 7 ++-----
>  fs/btrfs/extent_map.h | 2 +-
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 0cdb3e86f29b..f99c458071a4 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -292,10 +292,9 @@ static void try_merge_map(struct extent_map_tree *tree, struct extent_map *em)
>   * to the generation that actually added the file item to the inode so we know
>   * we need to sync this extent when we call fsync().
>   */
> -int unpin_extent_cache(struct extent_map_tree *tree, u64 start, u64 len,
> -		       u64 gen)
> +void unpin_extent_cache(struct extent_map_tree *tree, u64 start, u64 len,
> +			u64 gen)
>  {
> -	int ret = 0;
>  	struct extent_map *em;
>  	bool prealloc = false;
>  
> @@ -327,8 +326,6 @@ int unpin_extent_cache(struct extent_map_tree *tree, u64 start, u64 len,
>  	free_extent_map(em);
>  out:
>  	write_unlock(&tree->lock);
> -	return ret;
> -
>  }

This function has unfortunatelly attracting attention to do a simple fix
to just return void, several have been aleary sent but none of them
fixes it properly. To the point I don't want to reply anymore.

https://lore.kernel.org/linux-btrfs/20180815124425.GM24025@twin.jikos.cz/
https://lore.kernel.org/linux-btrfs/20230530150359.GS575@twin.jikos.cz/
https://patchwork.kernel.org/project/linux-btrfs/patch/20190416055739.25771-1-wqu@suse.com/#22596309

"When switching a fuction to return void, please check the whole
callgraph for functions that do not properly handler errors and do
BUG_ON. You won't see errors passed from them so this gives the
impression no error handling is needed in the caller."