xfs
[Top] [All Lists]

Re: [PATCH 3/3] kill xfs_dinode_core_t

To: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
Subject: Re: [PATCH 3/3] kill xfs_dinode_core_t
From: Christoph Hellwig <hch@xxxxxx>
Date: Wed, 8 Oct 2008 15:22:12 +0200
In-reply-to: <20081007213610.GV30001@disturbed>
References: <20081007202201.GC16485@xxxxxx> <20081007213610.GV30001@disturbed>
User-agent: Mutt/1.3.28i
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.

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