On Sat, Feb 02, 2019 at 08:27:37AM -0800, Christoph Hellwig wrote: > > + ASSERT(new_agino == NULLAGINO || > > + xfs_verify_agino(mp, XFS_INO_TO_AGNO(mp, ino), new_agino)); > > We have quite a few uses of this pattern. Shouldn't we just add a > a xfs_verify_agino_or_null helper? Yeah, I'll clean that up. > Also given that the caller has the agno and we use it a few times > maybe pass it as an argument? > > Also shouldn't new_agino be called something like next_agino or at > least have a little comment explaining what it stands for? > > > + xfs_trans_log_buf(tp, ibp, offset, (offset + sizeof(xfs_agino_t) - 1)); > > No need for the innter braces. > > Otherwise this looks conceptually fine to me. Ok, will fix this and the others. --D
