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

Alex Elder
2011-07-06T03:38:24Z

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@lst.de>

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@sgi.com>

. . .
}
> 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 */

. . .