qa: Remove all instances of remove_all except test cleanup

Adds a lint check for `remove_all()`

`fs::remove_all()`/`std::filesystem::remove_all()` is extremely
dangerous, all user-facing instances of it have been removed, and it
also deserves to be removed from the places in our test code where it is
being used unnecessarily.
This commit is contained in:
David Gumberg
2026-01-28 14:57:20 -08:00
parent 667e081a2a
commit a7e4a59d6d
6 changed files with 43 additions and 8 deletions

View File

@@ -44,16 +44,19 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted)
bilingual_str error_string;
std::vector<bilingual_str> warnings;
auto wallet_path = fs::PathToString(test_setup->m_path_root / "test_wallet");
const auto wallet_path = test_setup->m_path_root / "test_wallet";
const auto wallet_name = fs::PathToString(wallet_path);
bench.run([&] {
auto wallet = CreateWallet(context, wallet_path, /*load_on_start=*/std::nullopt, options, status, error_string, warnings);
auto wallet = CreateWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error_string, warnings);
assert(status == DatabaseStatus::SUCCESS);
assert(wallet != nullptr);
// Release wallet
RemoveWallet(context, wallet, /*load_on_start=*/ std::nullopt);
WaitForDeleteWallet(std::move(wallet));
fs::remove_all(wallet_path);
fs::remove(wallet_path / "wallet.dat");
fs::remove(wallet_path);
});
}

View File

@@ -61,7 +61,7 @@ FUZZ_TARGET(i2p, .init = initialize_i2p)
}
}
fs::remove_all(private_key_path);
fs::remove(private_key_path);
CreateSock = CreateSockOrig;
}

View File

@@ -1175,8 +1175,8 @@ BOOST_AUTO_TEST_CASE(btck_chainman_regtest_tests)
BOOST_CHECK_EQUAL(count, chain.CountEntries());
fs::remove_all(test_directory.m_directory / "blocks" / "blk00000.dat");
fs::remove(test_directory.m_directory / "blocks" / "blk00000.dat");
BOOST_CHECK(!chainman->ReadBlock(tip_2).has_value());
fs::remove_all(test_directory.m_directory / "blocks" / "rev00000.dat");
fs::remove(test_directory.m_directory / "blocks" / "rev00000.dat");
BOOST_CHECK_THROW(chainman->ReadBlockSpentOutputs(tip), std::runtime_error);
}

View File

@@ -1138,7 +1138,8 @@ BOOST_AUTO_TEST_CASE(test_LockDirectory)
#endif
// Clean up
ReleaseDirectoryLocks();
fs::remove_all(dirname);
fs::remove(dirname / lockname);
fs::remove(dirname);
}
BOOST_AUTO_TEST_CASE(test_ToLower)

View File

@@ -122,6 +122,32 @@ fs:: namespace, which has unsafe filesystem functions marked as deleted.
}
}
pub fn lint_remove_all() -> LintResult {
let found = git()
.args([
"grep",
"--line-number",
"remove_all(.*)",
"--",
"./src/",
// These are likely not avoidable.
":(exclude)src/test/kernel/test_kernel.cpp",
":(exclude)src/test/util/setup_common.cpp",
])
.status()
.expect("command error")
.success();
if found {
Err(r#"
Use of fs::remove_all or std::filesystem::remove_all is dangerous and should be avoided.
"#
.trim()
.to_string())
} else {
Ok(())
}
}
pub fn lint_rpc_assert() -> LintResult {
let found = git()
.args([

View File

@@ -14,7 +14,7 @@ use std::fs;
use std::process::{Command, ExitCode};
use lint_cpp::{
lint_boost_assert, lint_includes_build_config, lint_rpc_assert, lint_std_filesystem,
lint_boost_assert, lint_includes_build_config, lint_remove_all, lint_rpc_assert, lint_std_filesystem,
};
use lint_docs::{lint_doc_args, lint_doc_release_note_snippets, lint_markdown};
use lint_py::lint_py_lint;
@@ -57,6 +57,11 @@ fn get_linter_list() -> Vec<&'static Linter> {
name: "std_filesystem",
lint_fn: lint_std_filesystem
},
&Linter {
description: "Check that remove_all is not used",
name: "remove_all",
lint_fn: lint_remove_all
},
&Linter {
description: "Check that fatal assertions are not used in RPC code",
name: "rpc_assert",