| 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> |
|---|---|---|
| ||
| Previous by Date: | Re: [PATCH 16/27] xfs: avoid usage of struct xfs_dir2_block, Christoph Hellwig |
|---|---|
| Next by Date: | Re: [PATCH 18/27] xfs: avoid usage of struct xfs_dir2_data, Christoph Hellwig |
| Previous by Thread: | Re: [PATCH 17/27] xfs: kill struct xfs_dir2_block, Dave Chinner |
| Next by Thread: | Re: [PATCH 17/27] xfs: kill struct xfs_dir2_block, Alex Elder |
| Indexes: | [Date] [Thread] [Top] [All Lists] |