Re: [PATCH 12/21] implement generic xfs_btree_updkey

[Dave Chinner <david@xxxxxxxxxxxxx>]

On Tue, Jul 29, 2008 at 09:31:10PM +0200, Christoph Hellwig wrote:
> From: Dave Chinner <dgc@xxxxxxx>
> 
> 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@xxxxxx>

.....

> +/*
> + * 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@xxxxxxxxxxxxx