Re: [PATCH 09/21] implement generic xfs_btree_increment

[Dave Chinner <david@fromorbit.com>]

On Fri, Aug 01, 2008 at 09:40:00PM +0200, Christoph Hellwig wrote:
> On Wed, Jul 30, 2008 at 12:06:45PM +1000, Dave Chinner wrote:
> > > +int xfs_btree_increment(struct xfs_btree_cur *, int, int *);
> > 
> > I think for these interfaces it woul dbe better to include
> > variable names in the prototypes. i.e.:
> > 
> > int xfs_btree_increment(struct xfs_btree_cur *cur, int level, int *stat);
> 
> Well, all this and much more is documented at the implementation site,
> so I'd avoid this churn.  If there are strong feelings for it I
> can change the prototypes.

Not a big deal, really. I was just looking at consistency with the
other prototypes around this one....

> > Would it be better here to explicitly test this rather than assert?
> > ie.:
> > 
> > 	if (lev == cur->bc_nlevels) {
> > 		if (cur->bc_flags & XFS_BTREE_ROOT_IN_INODE)
> > 			goto out0;
> > 		ASSERT(0);
> > 		error = EFSCORRUPTED;
> > 		goto error0;
> > 	}
> > 
> > So that we get an explicit error reported in the production systems
> > rather than carrying on and dying a horrible death somewhere else....
> 
> Yes, this is much better.  But we're getting on a slipperly slope here
> to introduce too many improvements. For the initial patches I'd like
> to stay as close as possible to the old btree implementations.

Sure, but this particular piece of code is different to wthe original
btree anyway because it has to handle two different cases. better
to get it right first time, IMO.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com