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

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

On Thu, Dec 22, 2022 at 03:19:00PM +0800, Zorro Lang wrote:
> 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.

How about:

"For test cases that are not regression tests, e.g. functional tests or
maintainer tests, this helper suggests git commits that should be
applied to source trees to avoid test failures."

--D

> 
> > +_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
> > 
>