* [PATCH] xfs_db: fix inverted logic in error path
@ 2023-04-11 17:46 Darrick J. Wong
2023-04-11 23:59 ` Dave Chinner
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2023-04-11 17:46 UTC (permalink / raw)
To: cem; +Cc: linux-xfs
From: Darrick J. Wong <djwong@kernel.org>
smatch complains proceeding into the if body if leaf is a null pointer.
This is backwards, so correct that.
check.c:3455 process_leaf_node_dir_v2_int() warn: variable dereferenced before check 'leaf'
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..d5f3d225 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 (leaf && stale != be16_to_cpu(leaf->hdr.stale)) {
if (!sflag || v)
dbprintf(_("dir %lld block %d stale mismatch "
"%d/%d\n"),
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs_db: fix inverted logic in error path
2023-04-11 17:46 [PATCH] xfs_db: fix inverted logic in error path Darrick J. Wong
@ 2023-04-11 23:59 ` Dave Chinner
2023-04-12 21:43 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2023-04-11 23:59 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: cem, linux-xfs
On Tue, Apr 11, 2023 at 10:46:44AM -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.
> This is backwards, so correct that.
>
> check.c:3455 process_leaf_node_dir_v2_int() warn: variable dereferenced before check 'leaf'
>
> 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..d5f3d225 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 (leaf && stale != be16_to_cpu(leaf->hdr.stale)) {
> if (!sflag || v)
> dbprintf(_("dir %lld block %d stale mismatch "
> "%d/%d\n"),
Looks good. I'm surprised the compiler didn't warn about this
obviously broken code...
Reviewed-by: Dave Chinner <dchinner@redhat.com>
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs_db: fix inverted logic in error path
2023-04-11 23:59 ` Dave Chinner
@ 2023-04-12 21:43 ` Darrick J. Wong
0 siblings, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2023-04-12 21:43 UTC (permalink / raw)
To: Dave Chinner; +Cc: cem, linux-xfs
On Wed, Apr 12, 2023 at 09:59:03AM +1000, Dave Chinner wrote:
> On Tue, Apr 11, 2023 at 10:46:44AM -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.
> > This is backwards, so correct that.
> >
> > check.c:3455 process_leaf_node_dir_v2_int() warn: variable dereferenced before check 'leaf'
> >
> > 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..d5f3d225 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 (leaf && stale != be16_to_cpu(leaf->hdr.stale)) {
> > if (!sflag || v)
> > dbprintf(_("dir %lld block %d stale mismatch "
> > "%d/%d\n"),
>
> Looks good. I'm surprised the compiler didn't warn about this
> obviously broken code...
>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
NAK, this logic is actually trying to switch between the V4 and V5
directory format logic, but it's using the subtlety that @leaf3 is a
null pointer for a V4 fs and non-null for a V5 fs. Hence this check
really should be:
} else if (!leaf3 && stale != be16_to_cpu(leaf->hdr.stale)) {
Even though that looks *totally* wrong. But xfs_check is deprecated, so
fmeh.
So I'll go fix that and rerun through testing. I had thought this was
fine, but xfs/291 barfed up errors overnight.
--D
> --
> Dave Chinner
> david@fromorbit.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-04-12 21:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-11 17:46 [PATCH] xfs_db: fix inverted logic in error path Darrick J. Wong
2023-04-11 23:59 ` Dave Chinner
2023-04-12 21:43 ` 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;
as well as URLs for NNTP newsgroup(s).