On Thu, Nov 07, 2019 at 11:14:41PM -0800, Christoph Hellwig wrote: > On Thu, Nov 07, 2019 at 11:05:04PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > > > > Coverity points out that xfs_btree_islastblock calls > > xfs_btree_check_block, but doesn't act on an error return. This > > predicate has no answer if the btree is corrupt, so tweak the helper to > > be able to return errors, and then modify the one call site. > > Could we just kill xfs_btree_islastblock? It has pretty trivial, and > only has a single caller which only uses on of the two branches in the > function anyway. I'd rather leave it as a btree primitive, honestly. That said, "Is this cursor pointing to the last block on $level?" only makes sense if you've already performed a lookup (or seek) operation. If you've done that, you've already checked the block, right? So I think we could just get rid of the _check_block call on the grounds that we already did that as part of the lookup (or turn it into an ASSERT), and then this becomes a short enough function to try to make it a four line static inline predicate. Same result, but slightly better encapsulation. (Yeah yeah, it's C, we're all one big happy family of bits...) --D