[PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data

[Dave Chinner]

On Fri, Jul 01, 2011 at 05:43:39AM -0400, Christoph Hellwig wrote:
> In most places we can simply pass around and use the struct xfs_dir2_data_hdr,
> which is the first and most important member of struct xfs_dir2_data instead
> of the full structure.
> 
> Signed-off-by: Christoph Hellwig <hch at lst.de>

Only a couple of minor things.

....

> @@ -251,12 +258,13 @@ xfs_dir2_data_freeinsert(
>  	xfs_dir2_data_free_t	new;		/* new bestfree entry */
>  
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(be32_to_cpu(hdr->magic) == XFS_DIR2_DATA_MAGIC ||
> +	       be32_to_cpu(hdr->magic) == XFS_DIR2_BLOCK_MAGIC);
>  #endif

You kill the ifdef __KERNEL__ there.

> @@ -286,36 +294,36 @@ xfs_dir2_data_freeinsert(
>   */
>  STATIC void
>  xfs_dir2_data_freeremove(
> -	xfs_dir2_data_t		*d,		/* data block pointer */
> +	xfs_dir2_data_hdr_t	*hdr,		/* data block header */
>  	xfs_dir2_data_free_t	*dfp,		/* bestfree entry pointer */
>  	int			*loghead)	/* out: log data header */
>  {
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(be32_to_cpu(hdr->magic) == XFS_DIR2_DATA_MAGIC ||
> +	       be32_to_cpu(hdr->magic) == XFS_DIR2_BLOCK_MAGIC);
>  #endif

And there.

> @@ -335,23 +344,23 @@ xfs_dir2_data_freescan(
>  	char			*p;		/* current entry pointer */
>  
>  #ifdef __KERNEL__
> -	ASSERT(be32_to_cpu(d->hdr.magic) == XFS_DIR2_DATA_MAGIC ||
> -	       be32_to_cpu(d->hdr.magic) == XFS_DIR2_BLOCK_MAGIC);
> +	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
> +	       hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC));
>  #endif

I'll stop commenting on this now ;)

However, I have noticed that you've converted some of the magic
number compares to cpu_to_be32(XFS_DIR2_DATA_MAGIC) form and not
others. I'm not so concerned about the ASSERT()s, but some of the
real runtime checks are touched but not then not changed around.
Anyway, this can probably be done later as a separate cleanup.

>  			if (!needscan) {
> -				xfs_dir2_data_freeremove(d, dfp, needlogp);
> -				(void)xfs_dir2_data_freeinsert(d, newdup,
> +				xfs_dir2_data_freeremove(hdr, dfp, needlogp);
> +				(void)xfs_dir2_data_freeinsert(hdr, newdup,
>  					needlogp);
> -				(void)xfs_dir2_data_freeinsert(d, newdup2,
> +				(void)xfs_dir2_data_freeinsert(hdr, newdup2,
>  					needlogp);
>  			}
>  		}

Kill the (void) casts?

Otherwise looks good.

Reviewed-by: Dave Chinner <dchinner at redhat.com>
-- 
Dave Chinner
david at fromorbit.com