On Sat, Oct 12, 2013 at 02:42:54PM +0800, Li Zhong wrote:
> coverity scan 997010 reported following leak in repair/phase6.c
>
> 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(),
> so I also added a bmap cancel there.
>
> Signed-off-by: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
> ---
> repair/phase6.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/repair/phase6.c b/repair/phase6.c
> index a4ad7a3..0d88ad2 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -1310,6 +1310,8 @@ longform_dir2_rebuild(
> do_warn(
> _("space reservation failed (%d), filesystem may be out of space\n"),
> error);
> + libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
> + XFS_TRANS_ABORT);
As per the rest of the code in phase 6, the error handling here
should call "res_failed()" as we can't sanely recover from an ENOSPC
error during phase6.
> break;
> }
>
> @@ -1323,6 +1325,7 @@ longform_dir2_rebuild(
> do_warn(
> _("name create failed in ino %" PRIu64 " (%d), filesystem may be out of space\n"),
> ino, error);
> + libxfs_bmap_cancel(&flist);
> libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES |
> XFS_TRANS_ABORT);
> break;
There's another case exactly the same in that function you missed.
What you should probably do is stack the error handling cases at the
end of the function like:
....
return;
out_bmap_cancel:
libxfs_bmap_cancel(&flist);
out_trans_cancel:
libxfs_trans_cancel(tp, XFS_TRANS_RELEASE_LOG_RES | XFS_TRANS_ABORT);
return;
}
And convert all the error handling cases to jump to the appropriate
error handler.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
