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

Timothy Shimmin
2008-07-29T04:11:02Z

Christoph Hellwig wrote:
> 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
I'm not surprised :-)

> 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.
>
Oh okay, method in the madness ;-)

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

--Tim