On Sun, Feb 09, 2014 at 08:27:19PM -0600, Eric Sandeen wrote:
> Many/most callers of xfs_verify_cksum() pass bp->b_addr and
> BBTOB(bp->b_length) as the first 2 args. Add a helper
> which can just accept the bp and the crc offset, and work
> it out on its own, for brevity.
>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> I'm not wedded to this; seems helpful and cleaner, but
> if there's a reason not to, *shrug*
>
> fs/xfs/xfs_alloc.c | 7 +++----
> fs/xfs/xfs_attr_leaf.c | 3 +--
> fs/xfs/xfs_btree.c | 8 ++++----
> fs/xfs/xfs_cksum.h | 7 +++++++
> fs/xfs/xfs_da_btree.c | 3 +--
> fs/xfs/xfs_dir2_block.c | 3 +--
> fs/xfs/xfs_dir2_data.c | 3 +--
> fs/xfs/xfs_dir2_leaf.c | 3 +--
> fs/xfs/xfs_dir2_node.c | 3 +--
> fs/xfs/xfs_ialloc.c | 4 ++--
> fs/xfs/xfs_symlink_remote.c | 2 +-
> 11 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/fs/xfs/xfs_alloc.c b/fs/xfs/xfs_alloc.c
> index 9eab2df..ab33714 100644
> --- a/fs/xfs/xfs_alloc.c
> +++ b/fs/xfs/xfs_alloc.c
> @@ -485,8 +485,7 @@ xfs_agfl_read_verify(
> if (!xfs_sb_version_hascrc(&mp->m_sb))
> return;
>
> - agfl_ok = xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> - offsetof(struct xfs_agfl, agfl_crc));
> + agfl_ok = xfs_buf_verify_cksum(bp, offsetof(struct xfs_agfl, agfl_crc));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
vs
> + !xfs_buf_verify_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF)) ||
> ^^^^^^^^^^^^^^^^^^^^^^
I think that we also be consistent with the way we specify
the offset of the CRC field. Some of the code uses a #define, and
some doesn't. My preference would be to use the #define format
everywhere as it makes the code briefer and easier to read and it
gets rid of the long lines and wrapping that is done in places.
> diff --git a/fs/xfs/xfs_cksum.h b/fs/xfs/xfs_cksum.h
> index fad1676..f605d64 100644
> --- a/fs/xfs/xfs_cksum.h
> +++ b/fs/xfs/xfs_cksum.h
> @@ -60,4 +60,11 @@ xfs_verify_cksum(char *buffer, size_t length, unsigned long cksum_offset)
> return *(__le32 *)(buffer + cksum_offset) == xfs_end_cksum(crc);
> }
>
> +static inline int
> +xfs_buf_verify_cksum(struct xfs_buf *bp, unsigned long cksum_offset)
> +{
> + return xfs_verify_cksum(bp->b_addr, BBTOB(bp->b_length),
> + cksum_offset);
> +}
> +
This introduces a dependency between xfs_buf.h and xfs_cksum.h.
i.e. if we include xfs_cksum.h we now have to include xfs_buf.h
before it.
Even though it means we'll have to juggle header files in quite a
few files, I'd prefer that we swap the dependency order as
xfs_cksum.h is shared with userspace and used in places that don't
necessarily know (or should know) what an xfs_buf is....
Otherwise it's a good cleanup.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
