[PATCH] xfs_db: fix broken logic in error path

[PATCH] xfs_db: fix broken logic in error path

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

smatch complains proceeding into the if body if leaf is a null pointer:

check.c:3614 process_leaf_node_dir_v2_int() warn: variable dereferenced before check 'leaf' (see line 3518)

However, the logic here is misleading and broken -- what we're trying to
do is switch between the v4 and v5 variants of the directory check.
We're using @leaf3 being a null pointer (or not) to determine v4 vs. v5,
so the "!" part of the comparison is correct, but the variable used
(leaf) is not.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 db/check.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/check.c b/db/check.c
index 964756d0..fdf1f6a1 100644
--- a/db/check.c
+++ b/db/check.c
@@ -3452,7 +3452,7 @@ process_leaf_node_dir_v2_int(
 				 id->ino, dabno, stale,
 				 be16_to_cpu(leaf3->hdr.stale));
 		error++;
-	} else if (!leaf && stale != be16_to_cpu(leaf->hdr.stale)) {
+	} else if (!leaf3 && stale != be16_to_cpu(leaf->hdr.stale)) {
 		if (!sflag || v)
 			dbprintf(_("dir %lld block %d stale mismatch "
 				 "%d/%d\n"),

Re: [PATCH] xfs_db: fix broken logic in error path

[Carlos Maiolino]

On Thu, Apr 27, 2023 at 12:02:33PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> smatch complains proceeding into the if body if leaf is a null pointer:
> 
> check.c:3614 process_leaf_node_dir_v2_int() warn: variable dereferenced before check 'leaf' (see line 3518)
> 
> However, the logic here is misleading and broken -- what we're trying to
> do is switch between the v4 and v5 variants of the directory check.
> We're using @leaf3 being a null pointer (or not) to determine v4 vs. v5,
> so the "!" part of the comparison is correct, but the variable used
> (leaf) is not.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Looks correct.
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> ---
>  db/check.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/db/check.c b/db/check.c
> index 964756d0..fdf1f6a1 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -3452,7 +3452,7 @@ process_leaf_node_dir_v2_int(
>  				 id->ino, dabno, stale,
>  				 be16_to_cpu(leaf3->hdr.stale));
>  		error++;
> -	} else if (!leaf && stale != be16_to_cpu(leaf->hdr.stale)) {
> +	} else if (!leaf3 && stale != be16_to_cpu(leaf->hdr.stale)) {
>  		if (!sflag || v)
>  			dbprintf(_("dir %lld block %d stale mismatch "
>  				 "%d/%d\n"),

-- 
Carlos Maiolino