On Tue, Apr 30, 2024 at 02:49:13PM +0200, Christoph Hellwig wrote:
> Various routines dealing with shortform directories have a similar
> pre-condition ASSERT boilerplate that does look a bit weird.
>
> Remove the assert that the inode fork is non-NULL as it doesn't buy
> anything over the NULL pointer dereference if it is.
>
> Remove the duplicate i_disk_size ASSERT that uses the less precise
> location of the parent inode number over the one using
> xfs_dir2_sf_hdr_size().
>
> Remove the if_nextents assert in xfs_dir2_sf_to_block as that is implied
> by the local formt (and not checked by the other functions either).
format
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_dir2_block.c | 4 ----
> fs/xfs/libxfs/xfs_dir2_sf.c | 12 ++----------
> 2 files changed, 2 insertions(+), 14 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_block.c b/fs/xfs/libxfs/xfs_dir2_block.c
> index 0f93ed1a4a74f4..035a54dbdd7586 100644
> --- a/fs/xfs/libxfs/xfs_dir2_block.c
> +++ b/fs/xfs/libxfs/xfs_dir2_block.c
> @@ -1105,12 +1105,8 @@ xfs_dir2_sf_to_block(
> trace_xfs_dir2_sf_to_block(args);
>
> ASSERT(ifp->if_format == XFS_DINODE_FMT_LOCAL);
> - ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> -
> ASSERT(ifp->if_bytes == dp->i_disk_size);
> - ASSERT(oldsfp != NULL);
> ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(oldsfp->i8count));
> - ASSERT(dp->i_df.if_nextents == 0);
>
> /*
> * Copy the directory into a temporary buffer.
> diff --git a/fs/xfs/libxfs/xfs_dir2_sf.c b/fs/xfs/libxfs/xfs_dir2_sf.c
> index 17a20384c8b719..1cd5228e1ce6af 100644
> --- a/fs/xfs/libxfs/xfs_dir2_sf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_sf.c
> @@ -378,9 +378,7 @@ xfs_dir2_sf_addname(
>
> ASSERT(xfs_dir2_sf_lookup(args) == -ENOENT);
> ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
> - ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
> - ASSERT(sfp != NULL);
> ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
> /*
> * Compute entry (and change in) size.
> @@ -855,9 +853,7 @@ xfs_dir2_sf_lookup(
> xfs_dir2_sf_check(args);
>
> ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
> - ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
> - ASSERT(sfp != NULL);
> ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
> /*
> * Special case for .
> @@ -920,21 +916,19 @@ xfs_dir2_sf_removename(
> struct xfs_inode *dp = args->dp;
> struct xfs_mount *mp = dp->i_mount;
> struct xfs_dir2_sf_hdr *sfp = dp->i_df.if_data;
> + int oldsize = dp->i_disk_size;
> int byteoff; /* offset of removed entry */
> int entsize; /* this entry's size */
> int i; /* shortform entry index */
> int newsize; /* new inode size */
> - int oldsize; /* old inode size */
> xfs_dir2_sf_entry_t *sfep; /* shortform directory entry */
>
> trace_xfs_dir2_sf_removename(args);
>
> ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
> - oldsize = (int)dp->i_disk_size;
> - ASSERT(oldsize >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == oldsize);
> - ASSERT(sfp != NULL);
> ASSERT(oldsize >= xfs_dir2_sf_hdr_size(sfp->i8count));
> +
> /*
> * Loop over the old directory entries.
> * Find the one we're deleting.
> @@ -1028,9 +1022,7 @@ xfs_dir2_sf_replace(
> trace_xfs_dir2_sf_replace(args);
>
> ASSERT(dp->i_df.if_format == XFS_DINODE_FMT_LOCAL);
> - ASSERT(dp->i_disk_size >= offsetof(struct xfs_dir2_sf_hdr, parent));
> ASSERT(dp->i_df.if_bytes == dp->i_disk_size);
> - ASSERT(sfp != NULL);
> ASSERT(dp->i_disk_size >= xfs_dir2_sf_hdr_size(sfp->i8count));
>
> /*
> --
> 2.39.2
>
>
