public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Carlos Maiolino <cmaiolino@redhat.com>
To: Hou Tao <houtao1@huawei.com>
Cc: xfs@oss.sgi.com
Subject: Re: [PATCH] xfs: remove the magic numbers in xfs_btree_block-related len macros
Date: Thu, 23 Jun 2016 12:21:37 +0200	[thread overview]
Message-ID: <20160623102137.GH27894@redhat.com> (raw)
In-Reply-To: <1466651420-126472-1-git-send-email-houtao1@huawei.com>

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@huawei.com>
> ---
>  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@redhat.com>

> 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@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs

-- 
Carlos

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2016-06-23 10:21 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-23  3:10 [PATCH] xfs: remove the magic numbers in xfs_btree_block-related len macros Hou Tao
2016-06-23 10:21 ` Carlos Maiolino [this message]
2016-06-24  6:43   ` Hou Tao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160623102137.GH27894@redhat.com \
    --to=cmaiolino@redhat.com \
    --cc=houtao1@huawei.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox