Re: [PATCH 02/15] use the same btree_cur union member for alloc and inobt trees

Christoph Hellwig
2008-07-29T04:06:07Z

On Tue, Jul 29, 2008 at 01:55:03PM +1000, Timothy Shimmin wrote:
> Hi there,
>
> This was fine but just one thing which looked odd:
>
> Christoph Hellwig wrote:
> >
> > Index: linux-2.6-xfs/fs/xfs/xfs_btree.c
> > ===================================================================
> > --- linux-2.6-xfs.orig/fs/xfs/xfs_btree.c 2008-07-16 03:24:18.000000000 +0200
> > +++ linux-2.6-xfs/fs/xfs/xfs_btree.c 2008-07-16 03:24:19.000000000 +0200
> > @@ -570,6 +570,13 @@ xfs_btree_init_cursor(
> > cur->bc_private.a.agbp = agbp;
> > cur->bc_private.a.agno = agno;
> > break;
> > + case XFS_BTNUM_INO:
> > + /*
> > + * Inode allocation btree fields.
> > + */
> > + cur->bc_private.a.agbp = agbp;
> > + cur->bc_private.a.agno = agno;
> > + break;
> > case XFS_BTNUM_BMAP:
> > /*
> > * Bmap btree fields.
> > @@ -582,13 +589,6 @@ xfs_btree_init_cursor(
> > cur->bc_private.b.flags = 0;
> > cur->bc_private.b.whichfork = whichfork;
> > break;
> > - case XFS_BTNUM_INO:
> > - /*
> > - * Inode allocation btree fields.
> > - */
> > - cur->bc_private.i.agbp = agbp;
> > - cur->bc_private.i.agno = agno;
> > - break;
> > default:
>
> Could probably just add XFS_BNUM_INO to the case below
> (and modify the comment):

We could, and in fact that was my plan initially but I gave it up
because later we'd add the method table initialization which
would be different for the alloc vs inobt trees. I then later factored
these out into separate functions, so this whole switch goes away
a few patches later in the series.

Given that it would only cause churn in the series I'd prefer to
leave the patch as-is.