public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH V3 0/2] xfsprogs: fix covscan issues
@ 2022-12-01  9:34 cem
  2022-12-01  9:34 ` [PATCH 1/2] xfs_repair: Fix check_refcount() error path cem
  2022-12-01  9:34 ` [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() " cem
  0 siblings, 2 replies; 5+ messages in thread
From: cem @ 2022-12-01  9:34 UTC (permalink / raw)
  To: linux-xfs

From: Carlos Maiolino <cmaiolino@redhat.com>

Fix a couple of minor issues found by covscan

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>

Carlos Maiolino (2):
  xfs_repair: Fix check_refcount() error path
  xfs_repair: Fix rmaps_verify_btree() error path

 repair/rmap.c | 43 ++++++++++++++++++++-----------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] xfs_repair: Fix check_refcount() error path
  2022-12-01  9:34 [PATCH V3 0/2] xfsprogs: fix covscan issues cem
@ 2022-12-01  9:34 ` cem
  2022-12-01 15:33   ` Darrick J. Wong
  2022-12-01  9:34 ` [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() " cem
  1 sibling, 1 reply; 5+ messages in thread
From: cem @ 2022-12-01  9:34 UTC (permalink / raw)
  To: linux-xfs

From: Carlos Maiolino <cmaiolino@redhat.com>

Add proper exit error paths to avoid checking all pointers at the current path

Fixes-coverity-id: 1512651

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
V2:
	- Rename error label from err_agf to err_pag
	- pass error directly to libxfs_btree_del_cursor() without
	  using ternary operator
V3:
	- Rename the remaining 2 err labels to match what they are freeing

 repair/rmap.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/repair/rmap.c b/repair/rmap.c
index 2c809fd4f..9ec5e9e13 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1379,7 +1379,7 @@ check_refcounts(
 	if (error) {
 		do_warn(_("Could not read AGF %u to check refcount btree.\n"),
 				agno);
-		goto err;
+		goto err_pag;
 	}
 
 	/* Leave the per-ag data "uninitialized" since we rewrite it later */
@@ -1388,7 +1388,7 @@ check_refcounts(
 	bt_cur = libxfs_refcountbt_init_cursor(mp, NULL, agbp, pag);
 	if (!bt_cur) {
 		do_warn(_("Not enough memory to check refcount data.\n"));
-		goto err;
+		goto err_agf;
 	}
 
 	rl_rec = pop_slab_cursor(rl_cur);
@@ -1401,7 +1401,7 @@ check_refcounts(
 			do_warn(
 _("Could not read reference count record for (%u/%u).\n"),
 					agno, rl_rec->rc_startblock);
-			goto err;
+			goto err_cur;
 		}
 		if (!have) {
 			do_warn(
@@ -1416,7 +1416,7 @@ _("Missing reference count record for (%u/%u) len %u count %u\n"),
 			do_warn(
 _("Could not read reference count record for (%u/%u).\n"),
 					agno, rl_rec->rc_startblock);
-			goto err;
+			goto err_cur;
 		}
 		if (!i) {
 			do_warn(
@@ -1446,14 +1446,12 @@ next_loop:
 		rl_rec = pop_slab_cursor(rl_cur);
 	}
 
-err:
-	if (bt_cur)
-		libxfs_btree_del_cursor(bt_cur, error ? XFS_BTREE_ERROR :
-							XFS_BTREE_NOERROR);
-	if (pag)
-		libxfs_perag_put(pag);
-	if (agbp)
-		libxfs_buf_relse(agbp);
+err_cur:
+	libxfs_btree_del_cursor(bt_cur, error);
+err_agf:
+	libxfs_buf_relse(agbp);
+err_pag:
+	libxfs_perag_put(pag);
 	free_slab_cursor(&rl_cur);
 }
 
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() error path
  2022-12-01  9:34 [PATCH V3 0/2] xfsprogs: fix covscan issues cem
  2022-12-01  9:34 ` [PATCH 1/2] xfs_repair: Fix check_refcount() error path cem
@ 2022-12-01  9:34 ` cem
  2022-12-01 15:33   ` Darrick J. Wong
  1 sibling, 1 reply; 5+ messages in thread
From: cem @ 2022-12-01  9:34 UTC (permalink / raw)
  To: linux-xfs

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>
---
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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] xfs_repair: Fix check_refcount() error path
  2022-12-01  9:34 ` [PATCH 1/2] xfs_repair: Fix check_refcount() error path cem
@ 2022-12-01 15:33   ` Darrick J. Wong
  0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2022-12-01 15:33 UTC (permalink / raw)
  To: cem; +Cc: linux-xfs

On Thu, Dec 01, 2022 at 10:34:07AM +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: 1512651
> 
> 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
> 	- pass error directly to libxfs_btree_del_cursor() without
> 	  using ternary operator
> V3:
> 	- Rename the remaining 2 err labels to match what they are freeing
> 
>  repair/rmap.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/repair/rmap.c b/repair/rmap.c
> index 2c809fd4f..9ec5e9e13 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -1379,7 +1379,7 @@ check_refcounts(
>  	if (error) {
>  		do_warn(_("Could not read AGF %u to check refcount btree.\n"),
>  				agno);
> -		goto err;
> +		goto err_pag;
>  	}
>  
>  	/* Leave the per-ag data "uninitialized" since we rewrite it later */
> @@ -1388,7 +1388,7 @@ check_refcounts(
>  	bt_cur = libxfs_refcountbt_init_cursor(mp, NULL, agbp, pag);
>  	if (!bt_cur) {
>  		do_warn(_("Not enough memory to check refcount data.\n"));
> -		goto err;
> +		goto err_agf;
>  	}
>  
>  	rl_rec = pop_slab_cursor(rl_cur);
> @@ -1401,7 +1401,7 @@ check_refcounts(
>  			do_warn(
>  _("Could not read reference count record for (%u/%u).\n"),
>  					agno, rl_rec->rc_startblock);
> -			goto err;
> +			goto err_cur;
>  		}
>  		if (!have) {
>  			do_warn(
> @@ -1416,7 +1416,7 @@ _("Missing reference count record for (%u/%u) len %u count %u\n"),
>  			do_warn(
>  _("Could not read reference count record for (%u/%u).\n"),
>  					agno, rl_rec->rc_startblock);
> -			goto err;
> +			goto err_cur;
>  		}
>  		if (!i) {
>  			do_warn(
> @@ -1446,14 +1446,12 @@ next_loop:
>  		rl_rec = pop_slab_cursor(rl_cur);
>  	}
>  
> -err:
> -	if (bt_cur)
> -		libxfs_btree_del_cursor(bt_cur, error ? XFS_BTREE_ERROR :
> -							XFS_BTREE_NOERROR);
> -	if (pag)
> -		libxfs_perag_put(pag);
> -	if (agbp)
> -		libxfs_buf_relse(agbp);
> +err_cur:
> +	libxfs_btree_del_cursor(bt_cur, error);
> +err_agf:
> +	libxfs_buf_relse(agbp);
> +err_pag:
> +	libxfs_perag_put(pag);
>  	free_slab_cursor(&rl_cur);
>  }
>  
> -- 
> 2.30.2
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() error path
  2022-12-01  9:34 ` [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() " cem
@ 2022-12-01 15:33   ` Darrick J. Wong
  0 siblings, 0 replies; 5+ messages in thread
From: Darrick J. Wong @ 2022-12-01 15:33 UTC (permalink / raw)
  To: cem; +Cc: linux-xfs

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
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-12-01 15:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-01  9:34 [PATCH V3 0/2] xfsprogs: fix covscan issues cem
2022-12-01  9:34 ` [PATCH 1/2] xfs_repair: Fix check_refcount() error path cem
2022-12-01 15:33   ` Darrick J. Wong
2022-12-01  9:34 ` [PATCH 2/2] xfs_repair: Fix rmaps_verify_btree() " cem
2022-12-01 15:33   ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox