xfs
[Top] [All Lists]

Re: [PATCH 5/6] always use struct xfs_btree_block instead of short / lon

To: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
Subject: Re: [PATCH 5/6] always use struct xfs_btree_block instead of short / longform structures
From: Christoph Hellwig <hch@xxxxxx>
Date: Tue, 16 Sep 2008 19:31:07 +0200
In-reply-to: <20080916062616.GY5811@disturbed>
References: <20080915004657.GF12213@xxxxxx> <20080916062616.GY5811@disturbed>
User-agent: Mutt/1.3.28i
On Tue, Sep 16, 2008 at 04:26:16PM +1000, Dave Chinner wrote:
> On Mon, Sep 15, 2008 at 02:46:57AM +0200, Christoph Hellwig wrote:
> > Always use the generic xfs_btree_block type instead of the short / long
> > structures.  Add XFS_BTREE_SBLOCK_LEN / XFS_BTREE_LBLOCK_LEN defines for
> > the length of a short / long form block.  The rationale for this is that
> > we will grow more btree block header variants to support CRCs and other
> > RAS information, and always accessing them through the same datatype
> > with unions for the short / long form pointers makes implementing this
> > much easier.
> .......
> > @@ -382,16 +382,16 @@ xfs_alloc_fixup_trees(
> >     }
> >  #ifdef DEBUG
> >     {
> > -           xfs_alloc_block_t       *bnoblock;
> > -           xfs_alloc_block_t       *cntblock;
> > +           struct xfs_btree_block          *bnoblock;
> > +           struct xfs_btree_block          *cntblock;
> 
> Only need one tab there?

I think this was aligning to something, but I don't really care.

> > -                   bnoblock = XFS_BUF_TO_ALLOC_BLOCK(bno_cur->bc_bufs[0]);
> > -                   cntblock = XFS_BUF_TO_ALLOC_BLOCK(cnt_cur->bc_bufs[0]);
> >                     XFS_WANT_CORRUPTED_RETURN(
> > -                           be16_to_cpu(bnoblock->bb_numrecs) ==
> > -                           be16_to_cpu(cntblock->bb_numrecs));
> > +                           bnoblock->bb_numrecs ==
> > +                           cntblock->bb_numrecs);
> 
> The comparison could probably be made one line....

That would be far over 80 characters.

> > +#define XFS_ALLOC_BLOCK_LEN(mp)    XFS_BTREE_SBLOCK_LEN
> > +
> > +/*
> >   * Record, key, and pointer address macros for btree blocks.
> > + * (note that some of these may appear unused, but they are used in 
> > userspace)
> 
> Shouldn't that comment go in one of the previous patches?
> (and others)

Yes.

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