On Wed, Apr 21, 2021 at 07:56:28AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 20, 2021 at 10:09:52AM -0700, Darrick J. Wong wrote:
> > On Mon, Apr 19, 2021 at 10:28:00AM +0200, Christoph Hellwig wrote:
> > > xfs_efi_log_item only looks at the embedded xfs_efi_log_item structure,
> > > so pass that directly and rename the function to xfs_efi_log_item_sizeof.
> > > This allows using the helper in xlog_recover_efi_commit_pass2 as well.
> > >
> > > Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> > > ---
> > > fs/xfs/xfs_extfree_item.c | 15 +++++++--------
> > > 1 file changed, 7 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
> > > index ed8d0790908ea7..7ae570d1944590 100644
> > > --- a/fs/xfs/xfs_extfree_item.c
> > > +++ b/fs/xfs/xfs_extfree_item.c
> > > @@ -70,11 +70,11 @@ xfs_efi_release(
> > > * structure.
> > > */
> > > static inline int
> > > -xfs_efi_item_sizeof(
> > > - struct xfs_efi_log_item *efip)
> > > +xfs_efi_log_item_sizeof(
> >
> > Shouldn't this be named xfs_efi_log_format_sizeof to correspond to the
> > name of the structure? Two patches from now you (re)introduce
> > xfs_efi_item_sizeof that returns the size of a struct xfs_efi_log_item,
> > which is confusing.
>
> Well, that was the main reason to move these out of place, as they are
> rather misnamed.
I agree with the motivation, but not the names you've picked. I don't
like xfs_efi_log_item_sizeof /not/ being the sizing function for struct
xfs_efi_log_item, because (in my head anyway) XXX_sizeof should be the
function for struct XXX.
IOWs I think that in the end these two helpers should be:
static inline int
xfs_efi_log_format_sizeof(
struct xfs_efi_log_format *elf)
{
return sizeof(*elf) + elf->efi_nextents * sizeof(struct xfs_extent);
}
static inline int
xfs_efi_log_item_sizeof(unsigned int nextents)
{
return sizeof(struct xfs_efi_log_item) + nextents * sizeof(struct xfs_extent);
}
--D
