Re: [PATCH 12/21] implement generic xfs_btree_updkey

[Dave Chinner <david@fromorbit.com>]

On Tue, Jul 29, 2008 at 09:31:10PM +0200, Christoph Hellwig wrote:
> From: Dave Chinner <dgc@sgi.com>
> 
> Note that there are many > 80 char lines introduced due to the
> xfs_btree_key casts.  But the places where this happens is throw-away
> code once the whole btree code gets merged into a common implementation.
> 
> The same is true for the temporary xfs_alloc_log_keys define to the new
> name.  All old users will be gone after a few patches.
> 
> [hch: split out from bigger patch and minor adaptions]
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

.....

> +/*
> + * Update keys at all levels from here to the root along the cursor's path.
> + */
> +int
> +xfs_btree_updkey(
> +	struct xfs_btree_cur	*cur,
> +	union xfs_btree_key	*keyp,
> +	int			level)
> +{
> +	struct xfs_btree_block	*block;
> +	struct xfs_buf		*bp;
> +	union xfs_btree_key	*kp;
> +	int			ptr;
> +#ifdef DEBUG
> +	int			error;
> +#endif

This can be scoped inside the for loop.

> +
> +	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> +	XFS_BTREE_TRACE_ARGIK(cur, level, keyp);
> +
> +	ASSERT(!(cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) || level >= 1);
> +
> +	/*
> +	 * Go up the tree from this level toward the root.
> +	 * At each level, update the key value to the value input.
> +	 * Stop when we reach a level where the cursor isn't pointing
> +	 * at the first entry in the block.
> +	 */
> +	for (ptr = 1; ptr == 1 && level < cur->bc_nlevels; level++) {
> +		block = xfs_btree_get_block(cur, level, &bp);
> +#ifdef DEBUG
> +		error = xfs_btree_check_block(cur, block, level, bp);
> +		if (error) {
> +			XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
> +			return error;
> +		}
> +#endif

And even then I think we might not need an error variable - it can
only return EFSCORRUPTED, so:

#ifdef DEBUG
		if (xfs_btree_check_block(cur, block, level, bp)) {
			XFS_BTREE_TRACE_CURSOR(cur, XBT_ERROR);
			return EFSCORRUPTED;
		}
#endif

Would remove the need for the error variable.

Otherwise looks ok.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com