xfs
[Top] [All Lists]

Re: [PATCH 17/27] xfs: kill struct xfs_dir2_block

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 17/27] xfs: kill struct xfs_dir2_block
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 6 Jul 2011 04:37:14 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20110706023157.GL1026@dastard>
References: <20110701094321.936534538@xxxxxxxxxxxxxxxxxxxxxx> <20110701094605.827598983@xxxxxxxxxxxxxxxxxxxxxx> <20110706023157.GL1026@dastard>
User-agent: Mutt/1.5.21 (2010-09-15)
On Wed, Jul 06, 2011 at 12:31:57PM +1000, Dave Chinner wrote:
> >     btp = xfs_dir2_block_tail_p(mp, hdr);
> > -   ptr = (char *)block->u;
> > +   ptr = (char *)(hdr + 1);
> >     endptr = (char *)xfs_dir2_block_leaf_p(btp);
> 
> That is slightly less obvious what it is doing. It's jumping over
> the entire header, but could easily be confused with jumping one
> byte in.
> 
> Perhaps adding a wrapper e.g. xfs_dir2_block_data_p(hdr) to match
> the xfs_dir2_block_tail_p() and xfs_dir2_block_leaf_p() wrappers,
> and converting all the other cases to use this as well?

I had that in the initial version, but given that we usually use
the result as char, and not one of the two types of the union just
made the code very messy.

I can try it again, maybe as a add-on patch at the end so that we can
decide if it actually improves anything.

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