Re: [PATCH] btrfs: Remove redundant code

[David Sterba <dsterba@suse.cz>]

On Wed, Jul 12, 2017 at 09:32:15AM +0300, Nikolay Borisov wrote:
> insert_into_bitmap has only one caller which always allocates the info struct
> passed. As such remove the any NULL checks for info and also remove code
> to allocate info in case it was NULL.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/free-space-cache.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index c5e6180cdb8c..fd24fb99d6dc 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -2101,7 +2101,7 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
>  		goto again;
>  
>  new_bitmap:
> -	if (info && info->bitmap) {
> +	if (info->bitmap) {
>  		add_new_bitmap(ctl, info, offset);
>  		added = 1;
>  		info = NULL;

What if we reach this point, go back to label "again:", come back to
this check again, then info would be NULL and dereferencing info->bitmap
would crash. Then the below code is still required.

> @@ -2109,17 +2109,6 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl *ctl,
>  	} else {
>  		spin_unlock(&ctl->tree_lock);
>  
> -		/* no pre-allocated info, allocate a new one */
> -		if (!info) {
> -			info = kmem_cache_zalloc(btrfs_free_space_cachep,
> -						 GFP_NOFS);
> -			if (!info) {
> -				spin_lock(&ctl->tree_lock);
> -				ret = -ENOMEM;
> -				goto out;
> -			}
> -		}
> -
>  		/* allocate the bitmap */
>  		info->bitmap = kzalloc(PAGE_SIZE, GFP_NOFS);
>  		spin_lock(&ctl->tree_lock);
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html