Re: [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() error path

From: "Darrick J. Wong" <djwong@kernel.org>
To: cem@kernel.org
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() error path
Date: Thu, 1 Dec 2022 07:33:40 -0800	[thread overview]
Message-ID: <Y4jJVFRxhHmgby/0@magnolia> (raw)
In-Reply-To: <20221201093408.87820-3-cem@kernel.org>

On Thu, Dec 01, 2022 at 10:34:08AM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cmaiolino@redhat.com>
> 
> Add proper exit error paths to avoid checking all pointers at the current path
> 
> Fixes-coverity-id: 1512654
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Looks good to me,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
> V2:
> 	- Rename error label from err_agf to err_pag
> V3:
> 	- Rename the remaining 2 err labels to match what they are freeing
> 
>  repair/rmap.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 9ec5e9e13..52106fd42 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -1004,7 +1004,7 @@ rmaps_verify_btree(
>  	if (error) {
>  		do_warn(_("Could not read AGF %u to check rmap btree.\n"),
>  				agno);
> -		goto err;
> +		goto err_pag;
>  	}
>  
>  	/* Leave the per-ag data "uninitialized" since we rewrite it later */
> @@ -1013,7 +1013,7 @@ rmaps_verify_btree(
>  	bt_cur = libxfs_rmapbt_init_cursor(mp, NULL, agbp, pag);
>  	if (!bt_cur) {
>  		do_warn(_("Not enough memory to check reverse mappings.\n"));
> -		goto err;
> +		goto err_agf;
>  	}
>  
>  	rm_rec = pop_slab_cursor(rm_cur);
> @@ -1023,7 +1023,7 @@ rmaps_verify_btree(
>  			do_warn(
>  _("Could not read reverse-mapping record for (%u/%u).\n"),
>  					agno, rm_rec->rm_startblock);
> -			goto err;
> +			goto err_cur;
>  		}
>  
>  		/*
> @@ -1039,7 +1039,7 @@ _("Could not read reverse-mapping record for (%u/%u).\n"),
>  				do_warn(
>  _("Could not read reverse-mapping record for (%u/%u).\n"),
>  						agno, rm_rec->rm_startblock);
> -				goto err;
> +				goto err_cur;
>  			}
>  		}
>  		if (!have) {
> @@ -1090,13 +1090,12 @@ next_loop:
>  		rm_rec = pop_slab_cursor(rm_cur);
>  	}
>  
> -err:
> -	if (bt_cur)
> -		libxfs_btree_del_cursor(bt_cur, XFS_BTREE_NOERROR);
> -	if (pag)
> -		libxfs_perag_put(pag);
> -	if (agbp)
> -		libxfs_buf_relse(agbp);
> +err_cur:
> +	libxfs_btree_del_cursor(bt_cur, XFS_BTREE_NOERROR);
> +err_agf:
> +	libxfs_buf_relse(agbp);
> +err_pag:
> +	libxfs_perag_put(pag);
>  	free_slab_cursor(&rm_cur);
>  }
>  
> -- 
> 2.30.2
> 