xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data
From: Alex Elder <aelder@xxxxxxx>
Date: Tue, 5 Jul 2011 22:38:22 -0500
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20110701094606.003170984@xxxxxxxxxxxxxxxxxxxxxx>
References: <20110701094321.936534538@xxxxxxxxxxxxxxxxxxxxxx> <20110701094606.003170984@xxxxxxxxxxxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
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 */

. . .



<Prev in Thread] Current Thread [Next in Thread>