Message ID | 1606984438-13997-1-git-send-email-joseph.qi@linux.alibaba.com (mailing list archive) |
---|---|
State | Deferred, archived |
Headers | show |
Series | xfs: remove unneeded return value check for xfs_rmapbt_init_cursor() | expand |
On Thu, Dec 03, 2020 at 04:33:58PM +0800, Joseph Qi wrote: > Since xfs_rmapbt_init_cursor() can always return a valid cursor, the > NULL check in caller is unneeded. > This also keeps the behavior consistent with other callers. > > Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> Looks good, Reviewed-by: Christoph Hellwig <hch@lst.de>
On Thu, Dec 03, 2020 at 04:33:58PM +0800, Joseph Qi wrote: > Since xfs_rmapbt_init_cursor() can always return a valid cursor, the > NULL check in caller is unneeded. > This also keeps the behavior consistent with other callers. > > Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> > --- > fs/xfs/libxfs/xfs_rmap.c | 9 --------- > fs/xfs/scrub/bmap.c | 5 ----- > fs/xfs/scrub/common.c | 2 -- > 3 files changed, 16 deletions(-) > > diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c > index 2668ebe..10e0cf99 100644 > --- a/fs/xfs/libxfs/xfs_rmap.c > +++ b/fs/xfs/libxfs/xfs_rmap.c > @@ -2404,10 +2404,6 @@ struct xfs_rmap_query_range_info { > return -EFSCORRUPTED; > > rcur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno); > - if (!rcur) { > - error = -ENOMEM; > - goto out_cur; > - } > } > *pcur = rcur; > > @@ -2446,11 +2442,6 @@ struct xfs_rmap_query_range_info { > error = -EFSCORRUPTED; > } > return error; > - > -out_cur: > - xfs_trans_brelse(tp, agbp); > - > - return error; > } > > /* > diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c > index fed56d2..dd165c0 100644 > --- a/fs/xfs/scrub/bmap.c > +++ b/fs/xfs/scrub/bmap.c > @@ -563,10 +563,6 @@ struct xchk_bmap_check_rmap_info { > return error; > > cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, agf, agno); > - if (!cur) { > - error = -ENOMEM; > - goto out_agf; > - } > > sbcri.sc = sc; > sbcri.whichfork = whichfork; > @@ -575,7 +571,6 @@ struct xchk_bmap_check_rmap_info { > error = 0; > > xfs_btree_del_cursor(cur, error); > -out_agf: > xfs_trans_brelse(sc->tp, agf); > return error; > } > diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c > index 1887605..6757dc7 100644 > --- a/fs/xfs/scrub/common.c > +++ b/fs/xfs/scrub/common.c > @@ -502,8 +502,6 @@ struct xchk_rmap_ownedby_info { > xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_RMAP)) { > sa->rmap_cur = xfs_rmapbt_init_cursor(mp, sc->tp, sa->agf_bp, > agno); > - if (!sa->rmap_cur) > - goto err; Would you mind cleaning out the other btree cursor allocation warts under fs/xfs/scrub/ ? --D > } > > /* Set up a refcountbt cursor for cross-referencing. */ > -- > 1.8.3.1 >
On 12/4/20 2:25 AM, Darrick J. Wong wrote: > On Thu, Dec 03, 2020 at 04:33:58PM +0800, Joseph Qi wrote: >> Since xfs_rmapbt_init_cursor() can always return a valid cursor, the >> NULL check in caller is unneeded. >> This also keeps the behavior consistent with other callers. >> >> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> >> --- >> fs/xfs/libxfs/xfs_rmap.c | 9 --------- >> fs/xfs/scrub/bmap.c | 5 ----- >> fs/xfs/scrub/common.c | 2 -- >> 3 files changed, 16 deletions(-) >> >> diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c >> index 2668ebe..10e0cf99 100644 >> --- a/fs/xfs/libxfs/xfs_rmap.c >> +++ b/fs/xfs/libxfs/xfs_rmap.c >> @@ -2404,10 +2404,6 @@ struct xfs_rmap_query_range_info { >> return -EFSCORRUPTED; >> >> rcur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno); >> - if (!rcur) { >> - error = -ENOMEM; >> - goto out_cur; >> - } >> } >> *pcur = rcur; >> >> @@ -2446,11 +2442,6 @@ struct xfs_rmap_query_range_info { >> error = -EFSCORRUPTED; >> } >> return error; >> - >> -out_cur: >> - xfs_trans_brelse(tp, agbp); >> - >> - return error; >> } >> >> /* >> diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c >> index fed56d2..dd165c0 100644 >> --- a/fs/xfs/scrub/bmap.c >> +++ b/fs/xfs/scrub/bmap.c >> @@ -563,10 +563,6 @@ struct xchk_bmap_check_rmap_info { >> return error; >> >> cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, agf, agno); >> - if (!cur) { >> - error = -ENOMEM; >> - goto out_agf; >> - } >> >> sbcri.sc = sc; >> sbcri.whichfork = whichfork; >> @@ -575,7 +571,6 @@ struct xchk_bmap_check_rmap_info { >> error = 0; >> >> xfs_btree_del_cursor(cur, error); >> -out_agf: >> xfs_trans_brelse(sc->tp, agf); >> return error; >> } >> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c >> index 1887605..6757dc7 100644 >> --- a/fs/xfs/scrub/common.c >> +++ b/fs/xfs/scrub/common.c >> @@ -502,8 +502,6 @@ struct xchk_rmap_ownedby_info { >> xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_RMAP)) { >> sa->rmap_cur = xfs_rmapbt_init_cursor(mp, sc->tp, sa->agf_bp, >> agno); >> - if (!sa->rmap_cur) >> - goto err; > > Would you mind cleaning out the other btree cursor allocation > warts under fs/xfs/scrub/ ? > Sure, I'll do this and send v2. Thanks, Joseph
diff --git a/fs/xfs/libxfs/xfs_rmap.c b/fs/xfs/libxfs/xfs_rmap.c index 2668ebe..10e0cf99 100644 --- a/fs/xfs/libxfs/xfs_rmap.c +++ b/fs/xfs/libxfs/xfs_rmap.c @@ -2404,10 +2404,6 @@ struct xfs_rmap_query_range_info { return -EFSCORRUPTED; rcur = xfs_rmapbt_init_cursor(mp, tp, agbp, agno); - if (!rcur) { - error = -ENOMEM; - goto out_cur; - } } *pcur = rcur; @@ -2446,11 +2442,6 @@ struct xfs_rmap_query_range_info { error = -EFSCORRUPTED; } return error; - -out_cur: - xfs_trans_brelse(tp, agbp); - - return error; } /* diff --git a/fs/xfs/scrub/bmap.c b/fs/xfs/scrub/bmap.c index fed56d2..dd165c0 100644 --- a/fs/xfs/scrub/bmap.c +++ b/fs/xfs/scrub/bmap.c @@ -563,10 +563,6 @@ struct xchk_bmap_check_rmap_info { return error; cur = xfs_rmapbt_init_cursor(sc->mp, sc->tp, agf, agno); - if (!cur) { - error = -ENOMEM; - goto out_agf; - } sbcri.sc = sc; sbcri.whichfork = whichfork; @@ -575,7 +571,6 @@ struct xchk_bmap_check_rmap_info { error = 0; xfs_btree_del_cursor(cur, error); -out_agf: xfs_trans_brelse(sc->tp, agf); return error; } diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c index 1887605..6757dc7 100644 --- a/fs/xfs/scrub/common.c +++ b/fs/xfs/scrub/common.c @@ -502,8 +502,6 @@ struct xchk_rmap_ownedby_info { xchk_ag_btree_healthy_enough(sc, sa->pag, XFS_BTNUM_RMAP)) { sa->rmap_cur = xfs_rmapbt_init_cursor(mp, sc->tp, sa->agf_bp, agno); - if (!sa->rmap_cur) - goto err; } /* Set up a refcountbt cursor for cross-referencing. */
Since xfs_rmapbt_init_cursor() can always return a valid cursor, the NULL check in caller is unneeded. This also keeps the behavior consistent with other callers. Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com> --- fs/xfs/libxfs/xfs_rmap.c | 9 --------- fs/xfs/scrub/bmap.c | 5 ----- fs/xfs/scrub/common.c | 2 -- 3 files changed, 16 deletions(-)