Re: [PATCH 31/46] xfs: cleanup xchk_dir_rec

Darrick J. Wong
2019-11-08T00:59:09Z

On Thu, Nov 07, 2019 at 07:23:55PM +0100, Christoph Hellwig wrote:
> Use an offset as the main means for iteration, and only do pointer
> arithmetics to find the data/unused entries.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/xfs/scrub/dir.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index bb08a1cbe523..501d60c9b09a 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -187,7 +187,8 @@ xchk_dir_rec(
> struct xfs_dir2_data_entry *dent;
> struct xfs_buf *bp;
> struct xfs_dir2_leaf_entry *ent;
> - char *p, *endp;
> + void *endp;
> + unsigned int offset;

Can this be named iter_off or something? There's already an @off variable
which is the offset-within-block that wa calculated from the entry pointer.

> xfs_ino_t ino;
> xfs_dablk_t rec_bno;
> xfs_dir2_db_t db;
> @@ -237,32 +238,31 @@ xchk_dir_rec(
> if (ds->sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
> goto out_relse;
>
> - dent = (struct xfs_dir2_data_entry *)(((char *)bp->b_addr) + off);
> + dent = bp->b_addr + off;
>
> /* Make sure we got a real directory entry. */
> - p = (char *)mp->m_dir_inode_ops->data_entry_p(bp->b_addr);
> + offset = mp->m_dir_inode_ops->data_entry_offset;
> endp = xfs_dir3_data_endp(mp->m_dir_geo, bp->b_addr);
> if (!endp) {
> xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
> goto out_relse;
> }
> - while (p < endp) {
> - struct xfs_dir2_data_entry *dep;
> - struct xfs_dir2_data_unused *dup;
> + for (;;) {
> + struct xfs_dir2_data_entry *dep = bp->b_addr + offset;
> + struct xfs_dir2_data_unused *dup = bp->b_addr + offset;
> +

Extra whitespace.

--D

> + if (offset >= endp - bp->b_addr) {
> + xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
> + goto out_relse;
> + }
>
> - dup = (struct xfs_dir2_data_unused *)p;
> if (be16_to_cpu(dup->freetag) == XFS_DIR2_DATA_FREE_TAG) {
> - p += be16_to_cpu(dup->length);
> + offset += be16_to_cpu(dup->length);
> continue;
> }
> - dep = (struct xfs_dir2_data_entry *)p;
> if (dep == dent)
> break;
> - p += mp->m_dir_inode_ops->data_entsize(dep->namelen);
> - }
> - if (p >= endp) {
> - xchk_fblock_set_corrupt(ds->sc, XFS_DATA_FORK, rec_bno);
> - goto out_relse;
> + offset += mp->m_dir_inode_ops->data_entsize(dep->namelen);
> }
>
> /* Retrieve the entry, sanity check it, and compare hashes. */
> --
> 2.20.1
>