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.
|