Re: [PATCH 3/3] kill xfs_dinode_core_t

[Christoph Hellwig <hch@lst.de>]

On Wed, Oct 08, 2008 at 08:36:10AM +1100, Dave Chinner wrote:
> >  	switch (dic->di_format) {
> >  	case XFS_DINODE_FMT_DEV:
> > -		buf->dt_rdev = be32_to_cpu(dip->di_u.di_dev);
> > +		buf->dt_rdev = be32_to_cpu(*(__be32 *)XFS_DFORK_DPTR(mp, dic));
> 
> That's not particularly obvious where the rdev value is stored.
> Perhaps a comment indicating that it's a direct dereference out
> of the start of the data area in the inode?

Yeah.  Or maybe I should add a nice accessor that does the casting
and endianess conversion.

> > +#define XFS_DINODE_CORE_SIZE(mp)	(96)
> > +#define XFS_DINODE_SIZE(mp)		(96 + sizeof(__be32))
> 
> probably shouldn't hard-code the second "96" there. Perhaps
> the XFS_DINODE_SIZE() macro should be a sizeof(xfs_dinode_t)?

Well, that will change once xfs_dinode grows.  But we could do it for
now and then use offsetoff of the first new member for the old one.

I'll see if I can find a way to make this a little more clear.