Re: [PATCH 22/26] move xfs_bmbt_killroot to common code

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 04, 2008 at 03:35:42AM +0200, Christoph Hellwig wrote:
> xfs_bmbt_killroot is a mostly generic implementation of moving from
> a real block based root to an inode based root.  So move it to xfs_btree.c
> where it can use all the nice infrastructure there and make it pointer
> size agnostic
> 
> The new name for it is xfs_btree_root_to_iroot which is not very
> nice but at least slightly more descriptive than the old name.
.....
> +
> +	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> +	level = cur->bc_nlevels - 1;
> +	ASSERT(level >= 1);

probably should assert root in inode is set here.

> +	cblock = xfs_btree_get_block(cur, level - 1, &cbp);
> +	numrecs = xfs_btree_get_numrecs(cblock);
> +
> +	if (numrecs > cur->bc_ops->get_dmaxrecs(cur, level))
> +	    	goto out0;
        ^^^^
Stray whitespace.

> +
> +	XFS_BTREE_STATS_INC(cur, killroot);
> +
> +#ifdef DEBUG
> +	xfs_btree_get_sibling(cur, block, &ptr, XFS_BB_LEFTSIB);
> +	ASSERT(xfs_btree_ptr_is_null(cur, &ptr));
> +	xfs_btree_get_sibling(cur, block, &ptr, XFS_BB_RIGHTSIB);
> +	ASSERT(xfs_btree_ptr_is_null(cur, &ptr));
> +
> +	// XXX(hch): this assert is bmap btree specific
> + 	ASSERT(cur->bc_ops->get_maxrecs(cur, level) ==
   ^
> +	       XFS_BMAP_BROOT_MAXRECS(ifp->if_broot_bytes));

Stray whitespace. As to the assert - what is it really trying to
check? That the btree root space in the inode is large enough to
fit the max number of records? If so, does it really need to be
checked here (i.e. could the caller do it?)

Otherwise looks ok.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com