On Thu, Jun 23, 2016 at 11:10:20AM +0800, Hou Tao wrote:
> replace the magic numbers by offsetof(...) and sizeof(...), and add two
> extra checks on xfs_check_ondisk_structs()
>
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_format.h | 66 ++++++++++++++++++++++++++++------------------
> fs/xfs/xfs_ondisk.h | 2 ++
> 2 files changed, 43 insertions(+), 25 deletions(-)
>
Particularly I liked the idea of defining the short and long block structures
outside of xfs_btree_block and removing magic numbers is a good thing too.
you can consider it
Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> diff --git a/fs/xfs/libxfs/xfs_format.h b/fs/xfs/libxfs/xfs_format.h
> index dc97eb21..d3069be 100644
> --- a/fs/xfs/libxfs/xfs_format.h
> +++ b/fs/xfs/libxfs/xfs_format.h
> @@ -1435,41 +1435,57 @@ typedef __be64 xfs_bmbt_ptr_t, xfs_bmdr_ptr_t;
> * with the crc feature bit, and all accesses to them must be conditional on
> * that flag.
> */
> +/* short form block */
> +struct xfs_btree_sblock_part {
> + __be32 bb_leftsib;
> + __be32 bb_rightsib;
> +
> + __be64 bb_blkno;
> + __be64 bb_lsn;
> + uuid_t bb_uuid;
> + __be32 bb_owner;
> + __le32 bb_crc;
> +};
> +
> +/* long form block */
> +struct xfs_btree_lblock_part {
> + __be64 bb_leftsib;
> + __be64 bb_rightsib;
> +
> + __be64 bb_blkno;
> + __be64 bb_lsn;
> + uuid_t bb_uuid;
> + __be64 bb_owner;
> + __le32 bb_crc;
> + __be32 bb_pad; /* padding for alignment */
> +};
> +
> struct xfs_btree_block {
> __be32 bb_magic; /* magic number for block type */
> __be16 bb_level; /* 0 is a leaf */
> __be16 bb_numrecs; /* current # of data records */
> union {
> - struct {
> - __be32 bb_leftsib;
> - __be32 bb_rightsib;
> -
> - __be64 bb_blkno;
> - __be64 bb_lsn;
> - uuid_t bb_uuid;
> - __be32 bb_owner;
> - __le32 bb_crc;
> - } s; /* short form pointers */
> - struct {
> - __be64 bb_leftsib;
> - __be64 bb_rightsib;
> -
> - __be64 bb_blkno;
> - __be64 bb_lsn;
> - uuid_t bb_uuid;
> - __be64 bb_owner;
> - __le32 bb_crc;
> - __be32 bb_pad; /* padding for alignment */
> - } l; /* long form pointers */
> + struct xfs_btree_sblock_part s;
> + struct xfs_btree_lblock_part l;
> } bb_u; /* rest */
> };
>
> -#define XFS_BTREE_SBLOCK_LEN 16 /* size of a short form block */
> -#define XFS_BTREE_LBLOCK_LEN 24 /* size of a long form block */
> +/* size of a short form block */
> +#define XFS_BTREE_SBLOCK_LEN \
> + (offsetof(struct xfs_btree_block, bb_u) + \
> + offsetof(struct xfs_btree_sblock_part, bb_blkno))
> +/* size of a long form block */
> +#define XFS_BTREE_LBLOCK_LEN \
> + (offsetof(struct xfs_btree_block, bb_u) + \
> + offsetof(struct xfs_btree_lblock_part, bb_blkno))
>
> /* sizes of CRC enabled btree blocks */
> -#define XFS_BTREE_SBLOCK_CRC_LEN (XFS_BTREE_SBLOCK_LEN + 40)
> -#define XFS_BTREE_LBLOCK_CRC_LEN (XFS_BTREE_LBLOCK_LEN + 48)
> +#define XFS_BTREE_SBLOCK_CRC_LEN \
> + (offsetof(struct xfs_btree_block, bb_u) + \
> + sizeof(struct xfs_btree_sblock_part))
> +#define XFS_BTREE_LBLOCK_CRC_LEN \
> + (offsetof(struct xfs_btree_block, bb_u) + \
> + sizeof(struct xfs_btree_lblock_part))
>
> #define XFS_BTREE_SBLOCK_CRC_OFF \
> offsetof(struct xfs_btree_block, bb_u.s.bb_crc)
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 184c44e..6f06c48 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -34,6 +34,8 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_key, 8);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmbt_rec, 16);
> XFS_CHECK_STRUCT_SIZE(struct xfs_bmdr_block, 4);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_sblock_part, 48);
> + XFS_CHECK_STRUCT_SIZE(struct xfs_btree_lblock_part, 64);
> XFS_CHECK_STRUCT_SIZE(struct xfs_btree_block, 72);
> XFS_CHECK_STRUCT_SIZE(struct xfs_dinode, 176);
> XFS_CHECK_STRUCT_SIZE(struct xfs_disk_dquot, 104);
> --
> 2.5.5
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
--
Carlos
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
