Re: [PATCH 19/25] xfs: add xfs_da_node verification

Dave Chinner
2012-10-31T00:49:00Z

On Tue, Oct 30, 2012 at 05:23:47PM -0700, Phil White wrote:
> On Wed, Oct 31, 2012 at 09:23:32AM +1100, Dave Chinner wrote:
> > There's also another problem with this - endian swapping is missing.
>
> Endian swapping doesn't matter. include/linux/types.h defines __be16 as
> a __u16 and 0 is 0 is 0, no matter which order you put the bytes.

Except that the compiler based endian checks throw a error because
it is wrong ;)

(make C=2 CF="-D__CHECK_ENDIAN__" fs/xfs/xfs.ko)

> Doesn't hurt to make it clear though.
>
> > xfs: add xfs_da_node verification
> >
> > From: Dave Chinner <dchinner@redhat.com>
> >
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> > fs/xfs/xfs_attr.c | 22 ++++------
> > fs/xfs/xfs_attr_leaf.c | 12 +++---
> > fs/xfs/xfs_attr_leaf.h | 8 ++--
> > fs/xfs/xfs_da_btree.c | 109 ++++++++++++++++++++++++++++++++++++------------
> > fs/xfs/xfs_da_btree.h | 3 ++
> > fs/xfs/xfs_dir2_leaf.c | 2 +-
> > fs/xfs/xfs_dir2_priv.h | 1 +
> > 7 files changed, 107 insertions(+), 50 deletions(-)
>
> I'm a little surprised (and dismayed) that it passed xfstests with that.
> Presumably, it never ran into a case where level or count were > 0 on
> an invalid xfs_da_node.

Sure, xfstests doesn't usually trigger directory corruption, and so
it never would have had a chance to fail the verification
incorrectly. More than anything, what we are testing with xfstests
is that the verification doesn't introduce regressions, not that the
verification is 100% correct. Review is what catches the thinkos and
typos (as it has in this case), so I think the process is working as
it should. ;)

Cheers,

Dave.

--
Dave Chinner
david@fromorbit.com