Re: [PATCH 1/1] xfs/122: fix EFI/EFD log format structure size after flex array conversion

[Zorro Lang <zlang@redhat.com>]

On Tue, Dec 20, 2022 at 04:21:42PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Adjust this test since made EFI/EFD log item format structs proper flex
> arrays instead of array[1].
> 
> This adjustment was made to the kernel source tree as part of a project
> to make the use of flex arrays more consistent throughout the kernel.
> Converting array[1] and array[0] to array[] also avoids bugs in various
> compiler ports that mishandle the array size computation.  Prior to the
> introduction of xfs_ondisk.h, these miscomputations resulted in kernels
> that would silently write out filesystem structures that would then not
> be recognized by more mainstream systems (e.g.  x86).
> 
> OFC nearly all those reports about buggy compilers are for tiny
> architectures that XFS doesn't work well on anyways, so in practice it
> hasn't created any user problems (AFAIK).
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

This version looks good to me, thanks for all these detailed information!

Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/rc         |   15 +++++++++++++++
>  tests/xfs/122     |    5 +++++
>  tests/xfs/122.out |    8 ++++----
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/common/rc b/common/rc
> index 8060c03b7d..67bd74dc89 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -1502,6 +1502,21 @@ _fixed_by_kernel_commit()
>  	_fixed_by_git_commit kernel $*
>  }
>  

I'd like to give some comments to the new _wants_* helpers when I merge
it (don't need send a new version again: ), to help others know the
different usage of _wants_* and _fixed_by_*. How about below comment:

# Compare with _fixed_by_* helpers, this helper is used for un-regression
# test case, e.g. xfs/122. Or a case would like to mention a git commit
# which is not a bug fix (maybe a default behavior/format change). Then
# use this helpers.

> +_wants_git_commit()
> +{
> +	local pkg=$1
> +	shift
> +
> +	echo "This test wants $pkg fix:" >> $seqres.hints
> +	echo "      $*" >> $seqres.hints
> +	echo >> $seqres.hints
> +}
> +

# Refer to _wants_git_commit

Feel free to make it better :)

Thanks,
Zorro

> +_wants_kernel_commit()
> +{
> +	_wants_git_commit kernel $*
> +}
> +
>  _check_if_dev_already_mounted()
>  {
>  	local dev=$1
> diff --git a/tests/xfs/122 b/tests/xfs/122
> index 91083d6036..e616f1987d 100755
> --- a/tests/xfs/122
> +++ b/tests/xfs/122
> @@ -17,6 +17,11 @@ _begin_fstest other auto quick clone realtime
>  _supported_fs xfs
>  _require_command "$INDENT_PROG" indent
>  
> +# Starting in Linux 6.1, the EFI log formats were adjusted away from using
> +# single-element arrays as flex arrays.
> +_wants_kernel_commit 03a7485cd701 \
> +	"xfs: fix memcpy fortify errors in EFI log format copying"
> +
>  # filter out known changes to xfs type sizes
>  _type_size_filter()
>  {
> diff --git a/tests/xfs/122.out b/tests/xfs/122.out
> index a56cbee84f..95e53c5081 100644
> --- a/tests/xfs/122.out
> +++ b/tests/xfs/122.out
> @@ -161,10 +161,10 @@ sizeof(xfs_disk_dquot_t) = 104
>  sizeof(xfs_dq_logformat_t) = 24
>  sizeof(xfs_dqblk_t) = 136
>  sizeof(xfs_dsb_t) = 264
> -sizeof(xfs_efd_log_format_32_t) = 28
> -sizeof(xfs_efd_log_format_64_t) = 32
> -sizeof(xfs_efi_log_format_32_t) = 28
> -sizeof(xfs_efi_log_format_64_t) = 32
> +sizeof(xfs_efd_log_format_32_t) = 16
> +sizeof(xfs_efd_log_format_64_t) = 16
> +sizeof(xfs_efi_log_format_32_t) = 16
> +sizeof(xfs_efi_log_format_64_t) = 16
>  sizeof(xfs_error_injection_t) = 8
>  sizeof(xfs_exntfmt_t) = 4
>  sizeof(xfs_exntst_t) = 4
>