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

Alex Elder
2011-07-06T15:11:51Z

On Wed, 2011-07-06 at 04:37 -0400, Christoph Hellwig wrote:
> 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.

I actually have become pretty accustomed to this
idiom and have a (small) preference for the way it
is here rather than using a new macro that does
the same thing. Either way is fine with me though.

-Alex

> > 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.
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs