[bug report] xfs: repair free space btrees

[Dan Carpenter <dan.carpenter@linaro.org>]

Hello Darrick J. Wong,

Commit 4bdfd7d15747 ("xfs: repair free space btrees") from Dec 15,
2023 (linux-next), leads to the following Smatch static checker
warning:

	fs/xfs/scrub/alloc_repair.c:781 xrep_abt_build_new_trees()
	warn: missing unwind goto?

fs/xfs/scrub/alloc_repair.c
    702 STATIC int
    703 xrep_abt_build_new_trees(
    704         struct xrep_abt                *ra)
    705 {
    706         struct xfs_scrub        *sc = ra->sc;
    707         struct xfs_btree_cur        *bno_cur;
    708         struct xfs_btree_cur        *cnt_cur;
    709         struct xfs_perag        *pag = sc->sa.pag;
    710         bool                        needs_resort = false;
    711         int                        error;
    712 
    713         /*
    714          * Sort the free extents by length so that we can set up the free space
    715          * btrees in as few extents as possible.  This reduces the amount of
    716          * deferred rmap / free work we have to do at the end.
    717          */
    718         error = xrep_cntbt_sort_records(ra, false);
    719         if (error)
    720                 return error;
    721 
    722         /*
    723          * Prepare to construct the new btree by reserving disk space for the
    724          * new btree and setting up all the accounting information we'll need
    725          * to root the new btree while it's under construction and before we
    726          * attach it to the AG header.
    727          */
    728         xrep_newbt_init_bare(&ra->new_bnobt, sc);
    729         xrep_newbt_init_bare(&ra->new_cntbt, sc);
    730 
    731         ra->new_bnobt.bload.get_records = xrep_abt_get_records;
    732         ra->new_cntbt.bload.get_records = xrep_abt_get_records;
    733 
    734         ra->new_bnobt.bload.claim_block = xrep_abt_claim_block;
    735         ra->new_cntbt.bload.claim_block = xrep_abt_claim_block;
    736 
    737         /* Allocate cursors for the staged btrees. */
    738         bno_cur = xfs_bnobt_init_cursor(sc->mp, NULL, NULL, pag);
    739         xfs_btree_stage_afakeroot(bno_cur, &ra->new_bnobt.afake);
    740 
    741         cnt_cur = xfs_cntbt_init_cursor(sc->mp, NULL, NULL, pag);
    742         xfs_btree_stage_afakeroot(cnt_cur, &ra->new_cntbt.afake);
    743 
    744         /* Last chance to abort before we start committing fixes. */
    745         if (xchk_should_terminate(sc, &error))
    746                 goto err_cur;
    747 
    748         /* Reserve the space we'll need for the new btrees. */
    749         error = xrep_abt_reserve_space(ra, bno_cur, cnt_cur, &needs_resort);
    750         if (error)
    751                 goto err_cur;
    752 
    753         /*
    754          * If we need to re-sort the free extents by length, do so so that we
    755          * can put the records into the cntbt in the correct order.
    756          */
    757         if (needs_resort) {
    758                 error = xrep_cntbt_sort_records(ra, needs_resort);
    759                 if (error)
    760                         goto err_cur;
    761         }
    762 
    763         /*
    764          * Due to btree slack factors, it's possible for a new btree to be one
    765          * level taller than the old btree.  Update the alternate incore btree
    766          * height so that we don't trip the verifiers when writing the new
    767          * btree blocks to disk.
    768          */
    769         pag->pagf_repair_bno_level = ra->new_bnobt.bload.btree_height;
    770         pag->pagf_repair_cnt_level = ra->new_cntbt.bload.btree_height;
    771 
    772         /* Load the free space by length tree. */
    773         ra->array_cur = XFARRAY_CURSOR_INIT;
    774         ra->longest = 0;
    775         error = xfs_btree_bload(cnt_cur, &ra->new_cntbt.bload, ra);
    776         if (error)
    777                 goto err_levels;
                        ^^^^^^^^^^^^^^^^
    778 
    779         error = xrep_bnobt_sort_records(ra);
    780         if (error)
--> 781                 return error;
                        ^^^^^^^^^^^^^
Should this be a goto err_levels?

    782 
    783         /* Load the free space by block number tree. */
    784         ra->array_cur = XFARRAY_CURSOR_INIT;
    785         error = xfs_btree_bload(bno_cur, &ra->new_bnobt.bload, ra);
    786         if (error)
    787                 goto err_levels;
    788 
    789         /*
    790          * Install the new btrees in the AG header.  After this point the old
    791          * btrees are no longer accessible and the new trees are live.
    792          */
    793         xfs_allocbt_commit_staged_btree(bno_cur, sc->tp, sc->sa.agf_bp);
    794         xfs_btree_del_cursor(bno_cur, 0);
    795         xfs_allocbt_commit_staged_btree(cnt_cur, sc->tp, sc->sa.agf_bp);
    796         xfs_btree_del_cursor(cnt_cur, 0);
    797 
    798         /* Reset the AGF counters now that we've changed the btree shape. */
    799         error = xrep_abt_reset_counters(ra);
    800         if (error)
    801                 goto err_newbt;
    802 
    803         /* Dispose of any unused blocks and the accounting information. */
    804         xrep_abt_dispose_reservations(ra, error);
    805 
    806         return xrep_roll_ag_trans(sc);
    807 
    808 err_levels:
    809         pag->pagf_repair_bno_level = 0;
    810         pag->pagf_repair_cnt_level = 0;
    811 err_cur:
    812         xfs_btree_del_cursor(cnt_cur, error);
    813         xfs_btree_del_cursor(bno_cur, error);
    814 err_newbt:
    815         xrep_abt_dispose_reservations(ra, error);
    816         return error;
    817 }

regards,
dan carpenter