Re: [PATCH 12/18] xfs: introduce the xfs_iext_cursor abstraction

[Christoph Hellwig <hch@lst.de>]

> > @@ -1553,8 +1558,6 @@ xfs_bmap_add_extent_delay_real(
> >  	nextents = (whichfork == XFS_COW_FORK ? &bma->ip->i_cnextents :
> >  						&bma->ip->i_d.di_nextents);

> > -	ASSERT(bma->idx >= 0);
> > -	ASSERT(bma->idx <= xfs_iext_count(ifp));
> 
> I think it might be useful to encapsulate this check (which is also part
> of xfs_iext_get_extent()) into a cursor validation helper so we can
> preserve these asserts (here and in the other bmap functions). E.g.,
> something like:
> 
> 	ASSERT(xfs_iext_valid(&bma->ext));

The bug problem here is that we don not check if it is valid, it just
is a plausability check.  bma->idx == xfs_iext_count(ifp) is not a
a valid index, but one beyond valid, a fact that the whole code
relies on.

But we do check for validity and plausability in the actual btree code,
so I don't think the additional checks here add much value.

> > - * Return true if there is an extent at index idx, and return the expanded
> > - * extent structure at idx in that case.  Else return false.
> > + * Return true if there is an extent at cursor cur and return the expanded
> > + * extent structure at cur in gotp in that case.  Else return false.
> 
> "Return true if the cursor points at an extent and return the extent
> structure in gotp. Else return false."

Fixed.

> >  bool		xfs_iext_lookup_extent(struct xfs_inode *ip,
> >  			struct xfs_ifork *ifp, xfs_fileoff_t bno,
> > -			xfs_extnum_t *idxp, struct xfs_bmbt_irec *gotp);
> > +			struct xfs_iext_cursor *cur,
> > +			 struct xfs_bmbt_irec *gotp);
> 
> Indentation looks off here.

Fixed.