Re: [PATCH 12/16] xfs: factor out a xfs_dir2_sf_addname_common helper

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

On Tue, Apr 30, 2024 at 02:49:22PM +0200, Christoph Hellwig wrote:
> Move the common code between the easy and hard cases into a common helper.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/xfs/libxfs/xfs_dir2_sf.c | 48 +++++++++++++++++++------------------
>  1 file changed, 25 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index 1e1dcdf83b8f95..43e1090082b45d 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -360,6 +360,27 @@ xfs_dir2_try_block_to_sf(
>  	return error;
>  }
>  
> +static void
> +xfs_dir2_sf_addname_common(
> +	struct xfs_da_args	*args,
> +	struct xfs_dir2_sf_entry *sfep,
> +	xfs_dir2_data_aoff_t	offset,
> +	bool			objchange)

Hmm.  @objchange==true in the _hard() case means that we already
converted the sf dir to ino8 format, right?  So that's why we don't need
to increment i8count?

If the answers are yes and yes, then
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


> +{
> +	struct xfs_inode	*dp = args->dp;
> +	struct xfs_mount	*mp = dp->i_mount;
> +	struct xfs_dir2_sf_hdr	*sfp = dp->i_df.if_data;
> +
> +	sfep->namelen = args->namelen;
> +	xfs_dir2_sf_put_offset(sfep, offset);
> +	memcpy(sfep->name, args->name, sfep->namelen);
> +	xfs_dir2_sf_put_ino(mp, sfp, sfep, args->inumber);
> +	xfs_dir2_sf_put_ftype(mp, sfep, args->filetype);
> +	sfp->count++;
> +	if (args->inumber > XFS_DIR2_MAX_SHORT_INUM && !objchange)
> +		sfp->i8count++;
> +}
> +
>  /*
>   * Add a name to a shortform directory.
>   * There are two algorithms, "easy" and "hard" which we decide on
> @@ -476,21 +497,7 @@ xfs_dir2_sf_addname_easy(
>  	 * Need to set up again due to realloc of the inode data.
>  	 */
>  	sfep = (xfs_dir2_sf_entry_t *)((char *)sfp + byteoff);
> -	/*
> -	 * Fill in the new entry.
> -	 */
> -	sfep->namelen = args->namelen;
> -	xfs_dir2_sf_put_offset(sfep, offset);
> -	memcpy(sfep->name, args->name, sfep->namelen);
> -	xfs_dir2_sf_put_ino(mp, sfp, sfep, args->inumber);
> -	xfs_dir2_sf_put_ftype(mp, sfep, args->filetype);
> -
> -	/*
> -	 * Update the header and inode.
> -	 */
> -	sfp->count++;
> -	if (args->inumber > XFS_DIR2_MAX_SHORT_INUM)
> -		sfp->i8count++;
> +	xfs_dir2_sf_addname_common(args, sfep, offset, false);
>  	dp->i_disk_size = new_isize;
>  	xfs_dir2_sf_check(args);
>  }
> @@ -562,17 +569,12 @@ xfs_dir2_sf_addname_hard(
>  	nbytes = (int)((char *)oldsfep - (char *)oldsfp);
>  	memcpy(sfp, oldsfp, nbytes);
>  	sfep = (xfs_dir2_sf_entry_t *)((char *)sfp + nbytes);
> +
>  	/*
>  	 * Fill in the new entry, and update the header counts.
>  	 */
> -	sfep->namelen = args->namelen;
> -	xfs_dir2_sf_put_offset(sfep, offset);
> -	memcpy(sfep->name, args->name, sfep->namelen);
> -	xfs_dir2_sf_put_ino(mp, sfp, sfep, args->inumber);
> -	xfs_dir2_sf_put_ftype(mp, sfep, args->filetype);
> -	sfp->count++;
> -	if (args->inumber > XFS_DIR2_MAX_SHORT_INUM && !objchange)
> -		sfp->i8count++;
> +	xfs_dir2_sf_addname_common(args, sfep, offset, objchange);
> +
>  	/*
>  	 * If there's more left to copy, do that.
>  	 */
> -- 
> 2.39.2
> 
>