Re: [PATCH 7/7] xfs: Replace one-element arrays with flexible-array members

["Darrick J. Wong" <djwong@kernel.org>]

On Mon, Apr 19, 2021 at 10:28:04AM +0200, Christoph Hellwig wrote:
> From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> 
> There is a regular need in the kernel to provide a way to declare having
> a dynamically sized set of trailing elements in a structure. Kernel code
> should always use “flexible array members”[1] for these cases. The older
> style of one-element or zero-length arrays should no longer be used[2].
> 
> Refactor the code according to the use of flexible-array members in
> multiple structures, instead of one-element arrays. Also, make use of
> the new struct_size() helper to properly calculate the size of some
> structures that contain flexible-array members.
> 
> [1] https://en.wikipedia.org/wiki/Flexible_array_member
> [2] https://www.kernel.org/doc/html/v5.9/process/deprecated.html#zero-length-and-one-element-arrays
> 
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> [hch: rebased on top of the previous cleanups]
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good to me besides the naming thing; with those resolved,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_log_format.h | 6 +++---
>  fs/xfs/xfs_extfree_item.c      | 9 +++------
>  fs/xfs/xfs_extfree_item.h      | 4 ++--
>  fs/xfs/xfs_ondisk.h            | 6 +++---
>  4 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_log_format.h b/fs/xfs/libxfs/xfs_log_format.h
> index 639035052b4f65..9b218c30659ad7 100644
> --- a/fs/xfs/libxfs/xfs_log_format.h
> +++ b/fs/xfs/libxfs/xfs_log_format.h
> @@ -559,7 +559,7 @@ struct xfs_efi_log_format {
>  	uint16_t		efi_size;	/* size of this item */
>  	uint32_t		efi_nextents;	/* # extents to free */
>  	uint64_t		efi_id;		/* efi identifier */
> -	struct xfs_extent	efi_extents[1];	/* array of extents to free */
> +	struct xfs_extent	efi_extents[];	/* array of extents to free */
>  };
>  
>  /*
> @@ -577,7 +577,7 @@ struct xfs_efi_log_format_32 {
>  	uint16_t		efi_size;	/* size of this item */
>  	uint32_t		efi_nextents;	/* # extents to free */
>  	uint64_t		efi_id;		/* efi identifier */
> -	struct xfs_extent_32	efi_extents[1];	/* array of extents to free */
> +	struct xfs_extent_32	efi_extents[];	/* array of extents to free */
>  } __attribute__((packed));
>  
>  /*
> @@ -590,7 +590,7 @@ struct xfs_efd_log_format {
>  	uint16_t		efd_size;	/* size of this item */
>  	uint32_t		efd_nextents;	/* # of extents freed */
>  	uint64_t		efd_efi_id;	/* id of corresponding efi */
> -	struct xfs_extent	efd_extents[1];	/* array of extents freed */
> +	struct xfs_extent	efd_extents[];	/* array of extents freed */
>  };
>  
>  /*
> diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> index a2abdfd3d076bf..8bea9c9ecf2042 100644
> --- a/fs/xfs/xfs_extfree_item.c
> +++ b/fs/xfs/xfs_extfree_item.c
> @@ -73,8 +73,7 @@ static inline int
>  xfs_efi_log_item_sizeof(
>  	struct xfs_efi_log_format *elf)
>  {
> -	return sizeof(*elf) +
> -	       (elf->efi_nextents - 1) * sizeof(struct xfs_extent);
> +	return struct_size(elf, efi_extents, elf->efi_nextents);
>  }
>  
>  STATIC void
> @@ -194,8 +193,7 @@ static inline int
>  xfs_efd_log_item_sizeof(
>  	struct xfs_efd_log_format *elf)
>  {
> -	return sizeof(struct xfs_efd_log_format) +
> -	       (elf->efd_nextents - 1) * sizeof(struct xfs_extent);
> +	return struct_size(elf, efd_extents, elf->efd_nextents);
>  }
>  
>  STATIC void
> @@ -636,8 +634,7 @@ xfs_efi_copy_format_32(
>  	struct xfs_efi_log_format_32	*src = buf->i_addr;
>  	unsigned int			i;
>  
> -	if (buf->i_len != sizeof(*src) +
> -	    (src->efi_nextents - 1) * sizeof(struct xfs_extent_32)) {
> +	if (buf->i_len != struct_size(src, efi_extents, src->efi_nextents)) {
>  		XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW, NULL);
>  		return -EFSCORRUPTED;
>  	}
> diff --git a/fs/xfs/xfs_extfree_item.h b/fs/xfs/xfs_extfree_item.h
> index 3bb62ef525f2e0..a01ce86145bb64 100644
> --- a/fs/xfs/xfs_extfree_item.h
> +++ b/fs/xfs/xfs_extfree_item.h
> @@ -55,7 +55,7 @@ struct xfs_efi_log_item {
>  static inline int xfs_efi_item_sizeof(unsigned int nextents)
>  {
>  	return sizeof(struct xfs_efi_log_item) +
> -		(nextents - 1) * sizeof(struct xfs_extent);
> +		nextents * sizeof(struct xfs_extent);
>  }
>  
>  /*
> @@ -73,7 +73,7 @@ struct xfs_efd_log_item {
>  static inline int xfs_efd_item_sizeof(unsigned int nextents)
>  {
>  	return sizeof(struct xfs_efd_log_item) +
> -		(nextents - 1) * sizeof(struct xfs_extent);
> +		nextents * sizeof(struct xfs_extent);
>  }
>  
>  /*
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index 739476f7dffa21..fa4b590671bf58 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -118,9 +118,9 @@ xfs_check_ondisk_structs(void)
>  	/* log structures */
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_buf_log_format,	88);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_dq_logformat,		24);
> -	XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format,	32);
> -	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format,	32);
> -	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_32,	28);
> +	XFS_CHECK_STRUCT_SIZE(struct xfs_efd_log_format,	16);
> +	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format,	16);
> +	XFS_CHECK_STRUCT_SIZE(struct xfs_efi_log_format_32,	16);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_extent,		16);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_extent_32,		12);
>  	XFS_CHECK_STRUCT_SIZE(struct xfs_log_dinode,		176);
> -- 
> 2.30.1
>