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@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>
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@xxxxxxxxxxxxx
