Re: [PATCH 41/46] xfs: cleanup xfs_dir2_data_entsize

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Nov 07, 2019 at 07:24:05PM +0100, Christoph Hellwig wrote:
> Remove the XFS_DIR2_DATA_ENTSIZE and XFS_DIR3_DATA_ENTSIZE and open
> code them in their only caller, which now becomes so simple that
> we can turn it into an inline function.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_da_format.c | 33 ---------------------------------
>  fs/xfs/libxfs/xfs_dir2_priv.h | 15 ++++++++++++++-
>  2 files changed, 14 insertions(+), 34 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_format.c b/fs/xfs/libxfs/xfs_da_format.c
> index 0e35e613fbf3..dd2389748672 100644
> --- a/fs/xfs/libxfs/xfs_da_format.c
> +++ b/fs/xfs/libxfs/xfs_da_format.c
> @@ -19,39 +19,6 @@
>   * Directory data block operations
>   */
>  
> -/*
> - * For special situations, the dirent size ends up fixed because we always know
> - * what the size of the entry is. That's true for the "." and "..", and
> - * therefore we know that they are a fixed size and hence their offsets are
> - * constant, as is the first entry.
> - *
> - * Hence, this calculation is written as a macro to be able to be calculated at
> - * compile time and so certain offsets can be calculated directly in the
> - * structure initaliser via the macro. There are two macros - one for dirents
> - * with ftype and without so there are no unresolvable conditionals in the
> - * calculations. We also use round_up() as XFS_DIR2_DATA_ALIGN is always a power
> - * of 2 and the compiler doesn't reject it (unlike roundup()).
> - */
> -#define XFS_DIR2_DATA_ENTSIZE(n)					\
> -	round_up((offsetof(struct xfs_dir2_data_entry, name[0]) + (n) +	\
> -		 sizeof(xfs_dir2_data_off_t)), XFS_DIR2_DATA_ALIGN)
> -
> -#define XFS_DIR3_DATA_ENTSIZE(n)					\
> -	round_up((offsetof(struct xfs_dir2_data_entry, name[0]) + (n) +	\
> -		 sizeof(xfs_dir2_data_off_t) + sizeof(uint8_t)),	\
> -		XFS_DIR2_DATA_ALIGN)
> -
> -int
> -xfs_dir2_data_entsize(
> -	struct xfs_mount	*mp,
> -	int			n)
> -{
> -	if (xfs_sb_version_hasftype(&mp->m_sb))
> -		return XFS_DIR3_DATA_ENTSIZE(n);
> -	else
> -		return XFS_DIR2_DATA_ENTSIZE(n);
> -}
> -
>  static uint8_t
>  xfs_dir2_data_get_ftype(
>  	struct xfs_dir2_data_entry *dep)
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index a6c3fb3a2f7b..54bbfdd6ad69 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -57,7 +57,6 @@ extern int xfs_dir2_leaf_to_block(struct xfs_da_args *args,
>  		struct xfs_buf *lbp, struct xfs_buf *dbp);
>  
>  /* xfs_dir2_data.c */
> -int xfs_dir2_data_entsize(struct xfs_mount *mp, int n);
>  __be16 *xfs_dir2_data_entry_tag_p(struct xfs_mount *mp,
>  		struct xfs_dir2_data_entry *dep);
>  
> @@ -172,4 +171,18 @@ extern xfs_failaddr_t xfs_dir2_sf_verify(struct xfs_inode *ip);
>  extern int xfs_readdir(struct xfs_trans *tp, struct xfs_inode *dp,
>  		       struct dir_context *ctx, size_t bufsize);
>  
> +static inline int
> +xfs_dir2_data_entsize(
> +	struct xfs_mount	*mp,
> +	int			namelen)

Why not unsigned int here?  Neither names nor entries can have negative
length.  Other than that, looks fine...

--D

> +{
> +	size_t			len;
> +
> +	len = offsetof(struct xfs_dir2_data_entry, name[0]) + namelen +
> +			sizeof(xfs_dir2_data_off_t) /* tag */;
> +	if (xfs_sb_version_hasftype(&mp->m_sb))
> +		len += sizeof(uint8_t);
> +	return round_up(len, XFS_DIR2_DATA_ALIGN);
> +}
> +
>  #endif /* __XFS_DIR2_PRIV_H__ */
> -- 
> 2.20.1
>