Re: [PATCH] btrfs: fixup error handling in fixup_inode_link_counts
From: David Sterba <hidden>
Date: 2021-05-21 12:41:02
On Wed, May 19, 2021 at 01:13:15PM -0400, Josef Bacik wrote:
quoted hunk ↗ jump to hunk
This function has the following pattern while (1) { ret = whatever(); if (ret) goto out; } ret = 0 out: return ret; However several places in this while loop we simply break; when there's a problem, thus clearing the return value, and in one case we do a return -EIO, and leak the memory for the path. Fix this by re-arranging the loop to deal with ret == 1 coming from btrfs_search_slot, and then simply delete the ret = 0; out: bit so everybody can break if there is an error, which will allow for proper error handling to occur. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/tree-log.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)diff --git a/fs/btrfs/tree-log.c b/fs/btrfs/tree-log.c index 18009095908b..f8f708c02462 100644 --- a/fs/btrfs/tree-log.c +++ b/fs/btrfs/tree-log.c@@ -1778,7 +1778,7 @@ static noinline int fixup_inode_link_counts(struct btrfs_trans_handle *trans, struct btrfs_root *root, struct btrfs_path *path) { - int ret; + int ret = 0; struct btrfs_key key; struct inode *inode;@@ -1791,6 +1791,7 @@ static noinline int fixup_inode_link_counts(struct btrfs_trans_handle *trans, break; if (ret == 1) { + ret = 0; if (path->slots[0] == 0) break; path->slots[0]--;@@ -1803,17 +1804,19 @@ static noinline int fixup_inode_link_counts(struct btrfs_trans_handle *trans, ret = btrfs_del_item(trans, root, path); if (ret) - goto out; + break; btrfs_release_path(path); inode = read_one_inode(root, key.offset); - if (!inode) - return -EIO; + if (!inode) { + ret = -EIO;
Not related to your patch, but forcing EIO does not seem to be right. read_one_inode is a simple wrapper around read_one_inode that returns ERR_PTR so we should perhaps return that
quoted hunk ↗ jump to hunk
+ break; + } ret = fixup_inode_link_count(trans, root, inode); iput(inode); if (ret) - goto out; + break; /* * fixup on a directory may create new entries,@@ -1822,8 +1825,6 @@ static noinline int fixup_inode_link_counts(struct btrfs_trans_handle *trans, */ key.offset = (u64)-1; } - ret = 0; -out: btrfs_release_path(path); return ret; }-- 2.26.3