xfs
[Top] [All Lists]

Re: [PATCH 00/26] generic btree implementation, version 3

To: Christoph Hellwig <hch@xxxxxx>
Subject: Re: [PATCH 00/26] generic btree implementation, version 3
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 5 Aug 2008 10:26:07 +1000
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20080804142131.GA9892@xxxxxx>
Mail-followup-to: Christoph Hellwig <hch@xxxxxx>, xfs@xxxxxxxxxxx
References: <20080804013158.GA8819@xxxxxx> <20080804015425.GE6119@disturbed> <20080804142131.GA9892@xxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Aug 04, 2008 at 04:21:32PM +0200, Christoph Hellwig wrote:
> On Mon, Aug 04, 2008 at 11:54:25AM +1000, Dave Chinner wrote:
> > > I'm not sure
> > > what to do with move_* - these are the most ugly helpers, so maybe
> > > we should just make them memmove wrappers in the style of copy_
> > > and leave all addressing to the callers.
> > 
> > Yes, it would be nice to have them use the same interface. If
> > we do that, then there's no real point for having a copy vs move
> > distinction - we could just make everything use the
> > memmove version and drop one of the interfaces altogether....
> 
> I've actually come up with another variant.  Since what we do in the
> memmove case is to move a number of entries in a single block up or down
> one position I've added the following helper:
> 
> STATIC void
> xfs_btree_shift_keys(
>       struct xfs_btree_cur    *cur,
>       union xfs_btree_key     *key
>       int                     dir,
>       int                     numkeys)
> {
>       char                    *dst_key;
> 
>       ASSERT(numkeys >= 0);
>       ASSERT(dir == 1 || dir == -1);
> 
>       dst_key = (char *)key + (dir * cur->bc_ops->key_len);
>       memmove(dst_key, key, numkeys * cur->bc_ops->key_len);
> }
> 
> and the same for ptrs and recs.  This follows the original code in
> spirit and is quite readable.

Nice. That fits nicely into the 'make a hole' or 'fill a hole' parts
of various functions which will remove a lot of magic from them.
The only thing I'd do is add an enum for the direction so the
callers are self-documenting as to the direction of the shift....

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx


<Prev in Thread] Current Thread [Next in Thread>