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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2011-07-01 at 05:43 -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@xxxxxx>

In xfs_dir2_data_freescan() you get the address of the
dir2_data_t by forcibly casting the address of the
header to that type.  We all know that's fine, but
it's an unsavory practice.  Why don't you pass the
full dir2_data_t in that case where it's needed?
Are you simply trying to avoid passing *any* pointers
to variable-sized types?

I'm not worried about this, but just wondered
what you thought about this.

(Update: I think you eliminate that line in the next
patch anyway...)

In any case:

Reviewed-by: Alex Elder <aelder@xxxxxxx>

. . .
		}
> Index: xfs/fs/xfs/xfs_dir2_data.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_dir2_data.c	2011-06-30 09:38:36.586734196 +0200
> +++ xfs/fs/xfs/xfs_dir2_data.c	2011-06-30 09:38:40.133400821 +0200

. . .
 
> @@ -325,9 +333,10 @@ xfs_dir2_data_freeremove(
>  void
>  xfs_dir2_data_freescan(
>  	xfs_mount_t		*mp,		/* filesystem mount point */
> -	xfs_dir2_data_t		*d,		/* data block pointer */
> +	xfs_dir2_data_hdr_t	*hdr,		/* data block header */
>  	int			*loghead)	/* out: log data header */
>  {
> +	xfs_dir2_data_t		*d = (xfs_dir2_data_t *)hdr;
>  	xfs_dir2_block_tail_t	*btp;		/* block tail */
>  	xfs_dir2_data_entry_t	*dep;		/* active data entry */
>  	xfs_dir2_data_unused_t	*dup;		/* unused data entry */

. . .



_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux