On 2016/6/23 18:21, Carlos Maiolino wrote:
> 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
>
I had tried to do more than just move two structs out of struct xfs_btree_block,
but the modification would be huge, so i just keep it simple.
> Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
>
Thanks for your review.
>> 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
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
