On Tue, Aug 05, 2008 at 11:05:57AM +1000, Dave Chinner wrote:
> I think that ->get_dmaxrecs is probably badly named. I called it
> that originally because it matched the macro name it was wrapping.
> Realisitically it should be ->get_root_maxrecs....
Yeah. The name giving macro is already gone in my current tree.
>
> > + /* Make a key out of the record data to be inserted, and save it. */
> > + cur->bc_ops->init_key_from_rec(cur, &key, recp);
> > +
> > + /* If we're off the left edge, return failure. */
> > + ptr = cur->bc_ptrs[level];
> > + if (ptr == 0) {
> > + XFS_BTREE_TRACE_CURSOR(cur, XBT_EXIT);
> > + *stat = 0;
> > + return 0;
> > + }
>
> You can probably move the key initialisation till after then initial
> 'in this block' checks.
Done.
> > + /*
> > + * If the block is full, we can't insert the new entry until we
> > + * make the block un-full.
> > + */
> > + xfs_btree_set_ptr_null(cur, &nptr);
> > + if (numrecs == cur->bc_ops->get_maxrecs(cur, level)) {
> > + error = xfs_btree_make_block_unfull(cur, level, numrecs,
> > + &optr, &ptr, &nptr, &ncur, &nrec, stat);
> > + if (error || *stat == 0)
> > + goto error0;
> > + }
> > +
> > + /* The current block may have changed during the split. */
> > + block = xfs_btree_get_block(cur, level, &bp);
> > + numrecs = xfs_btree_get_numrecs(block);
>
> The comment here should probably refer to the unfull call, not a
> 'split'. i.e.:
>
> /*
> * the current block may have changed if the block was
> * previously full and we have just made space in it.
> */
Updated.
|