On Thu, Jan 16, 2020 at 08:33:55AM -0800, Christoph Hellwig wrote:
> > - if (XFS_IS_CORRUPT(args->mp, !bp)) {
> > - error = -EFSCORRUPTED;
> > + error = xfs_btree_get_bufs(args->mp, args->tp, args->agno,
> > + fbno, &bp);
> > + if (error)
>
> Should we keep the XFS_IS_CORRUPT checks in some form? Not sure they
> matter all that much, though.
The XFS_IS_CORRUPT checks exist to report a corrupted filesystem, and
the only errors that the _get_buf* variants can return are runtime
errors like ENOMEM.
> > ASSERT(fsbno != NULLFSBLOCK);
> > d = XFS_FSB_TO_DADDR(mp, fsbno);
> > - error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
> > - if (error)
> > - return NULL;
> > - return bp;
> > + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
>
> Maybe kill the local variable while you're at it?
>
> > ASSERT(agno != NULLAGNUMBER);
> > ASSERT(agbno != NULLAGBLOCK);
> > d = XFS_AGB_TO_DADDR(mp, agno, agbno);
> > - error = xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, &bp);
> > - if (error)
> > - return NULL;
> > - return bp;
> > + return xfs_trans_get_buf(tp, mp->m_ddev_targp, d, mp->m_bsize, 0, bpp);
>
> Same here.
Will just get rid of them as you suggest later in this thread.
--D
