Skip to content

Commit

Permalink
btrfs: remove pointless and double ulist frees in error paths of qgro…
Browse files Browse the repository at this point in the history
…up tests

[ Upstream commit d0ea17a ]

Several places in the qgroup self tests follow the pattern of freeing the
ulist pointer they passed to btrfs_find_all_roots() if the call to that
function returned an error. That is pointless because that function always
frees the ulist in case it returns an error.

Also In some places like at test_multiple_refs(), after a call to
btrfs_qgroup_account_extent() we also leave "old_roots" and "new_roots"
pointing to ulists that were freed, because btrfs_qgroup_account_extent()
has freed those ulists, and if after that the next call to
btrfs_find_all_roots() fails, we call ulist_free() on the "old_roots"
ulist again, resulting in a double free.

So remove those calls to reduce the code size and avoid double ulist
free in case of an error.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
fdmanana authored and gregkh committed Nov 26, 2022
1 parent 8bef1dd commit 307f469
Showing 1 changed file with 4 additions and 12 deletions.
16 changes: 4 additions & 12 deletions fs/btrfs/tests/qgroup-tests.c
Expand Up @@ -225,7 +225,6 @@ static int test_no_shared_qgroup(struct btrfs_root *root,
*/
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &old_roots, false);
if (ret) {
ulist_free(old_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -240,7 +239,6 @@ static int test_no_shared_qgroup(struct btrfs_root *root,
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &new_roots, false);
if (ret) {
ulist_free(old_roots);
ulist_free(new_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -252,17 +250,18 @@ static int test_no_shared_qgroup(struct btrfs_root *root,
return ret;
}

/* btrfs_qgroup_account_extent() always frees the ulists passed to it. */
old_roots = NULL;
new_roots = NULL;

if (btrfs_verify_qgroup_counts(fs_info, BTRFS_FS_TREE_OBJECTID,
nodesize, nodesize)) {
test_err("qgroup counts didn't match expected values");
return -EINVAL;
}
old_roots = NULL;
new_roots = NULL;

ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &old_roots, false);
if (ret) {
ulist_free(old_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -276,7 +275,6 @@ static int test_no_shared_qgroup(struct btrfs_root *root,
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &new_roots, false);
if (ret) {
ulist_free(old_roots);
ulist_free(new_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand Down Expand Up @@ -326,7 +324,6 @@ static int test_multiple_refs(struct btrfs_root *root,

ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &old_roots, false);
if (ret) {
ulist_free(old_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -341,7 +338,6 @@ static int test_multiple_refs(struct btrfs_root *root,
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &new_roots, false);
if (ret) {
ulist_free(old_roots);
ulist_free(new_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -361,7 +357,6 @@ static int test_multiple_refs(struct btrfs_root *root,

ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &old_roots, false);
if (ret) {
ulist_free(old_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -376,7 +371,6 @@ static int test_multiple_refs(struct btrfs_root *root,
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &new_roots, false);
if (ret) {
ulist_free(old_roots);
ulist_free(new_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -402,7 +396,6 @@ static int test_multiple_refs(struct btrfs_root *root,

ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &old_roots, false);
if (ret) {
ulist_free(old_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand All @@ -417,7 +410,6 @@ static int test_multiple_refs(struct btrfs_root *root,
ret = btrfs_find_all_roots(&trans, fs_info, nodesize, 0, &new_roots, false);
if (ret) {
ulist_free(old_roots);
ulist_free(new_roots);
test_err("couldn't find old roots: %d", ret);
return ret;
}
Expand Down

0 comments on commit 307f469

Please sign in to comment.