[PATCH v2] xfsprogs: fix resource leak in longform_dir2_rebuild()

[Li Zhong <zhong@linux.vnet.ibm.com>]

coverity scan 997010 reported following leak:

1309                if (error) {
1310                        do_warn(
1311        _("space reservation failed (%d), filesystem may be out of space\n"),
1312                                error);
    	25. Breaking from loop
1313                        break;
1314                }

......

1342                libxfs_trans_commit(tp,
1343                                XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
1344        }

CID 997010 (#1 of 1): Resource leak (RESOURCE_LEAK)
26. leaked_storage: Variable "tp" going out of scope leaks the storage it points to.
1345}

Though not reported by coverity, it seems that there might be some entries in
flist which needs to be freed in the failure case below libxfs_dir_createname(),
and libxfs_bunmapi().

The fix cleans up the code by stacking the error handling at the end of the
function, and jumping to the error handler label for the above cases. (fail
directly by calling res_failed() for reservation failure.)

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
v2: as Dave suggested,
        use res_failed() for libxfs_trans_reserve()
        adding bmap cancel for one missed case
        using error handling lables to clean up the code

 repair/phase6.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/repair/phase6.c b/repair/phase6.c
index a4ad7a3..6b3fb09 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -1280,9 +1280,7 @@ longform_dir2_rebuild(
 				&firstblock, &flist, &done);
 	if (error) {
 		do_warn(_("xfs_bunmapi failed -- error - %d\n"), error);
-		libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
-					XFS_TRANS_ABORT);
-		return;
+		goto out_bmap_cancel;
 	}
 
 	ASSERT(done);
@@ -1306,12 +1304,8 @@ longform_dir2_rebuild(
 		nres = XFS_CREATE_SPACE_RES(mp, p->name.len);
 		error = libxfs_trans_reserve(tp, &M_RES(mp)->tr_create,
 					     nres, 0);
-		if (error) {
-			do_warn(
-	_("space reservation failed (%d), filesystem may be out of space\n"),
-				error);
-			break;
-		}
+		if (error)
+			res_failed(error);
 
 		libxfs_trans_ijoin(tp, ip, 0);
 		libxfs_trans_ihold(tp, ip);
@@ -1323,9 +1317,7 @@ longform_dir2_rebuild(
 			do_warn(
 _("name create failed in ino %" PRIu64 " (%d), filesystem may be out of space\n"),
 				ino, error);
-			libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
-						XFS_TRANS_ABORT);
-			break;
+			goto out_bmap_cancel;
 		}
 
 		error = libxfs_bmap_finish(&tp, &flist, &committed);
@@ -1333,15 +1325,19 @@ _("name create failed in ino %" PRIu64 " (%d), filesystem may be out of space\n"
 			do_warn(
 	_("bmap finish failed (%d), filesystem may be out of space\n"),
 				error);
-			libxfs_bmap_cancel(&flist);
-			libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
-						XFS_TRANS_ABORT);
-			break;
+			goto out_bmap_cancel;
 		}
 
 		libxfs_trans_commit(tp,
 				XFS_TRANS_RELEASE_LOG_RES|XFS_TRANS_SYNC);
 	}
+
+	return;
+
+out_bmap_cancel:
+	libxfs_bmap_cancel(&flist);
+	libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
+	return;
 }
 
 
-- 
1.8.1.4



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs