refactor: Drop unused fclose() from BufferedFile

This was only explicitly used in the tests, where it can be replaced by
wrapping the original raw file pointer into a CAutoFile on creation and
then calling CAutoFile::fclose().

Also, it was used in LoadExternalBlockFile(), where it can also be
replaced by the (implicit call to the) CAutoFile destructor after
wrapping the original raw file pointer in a CAutoFile.
This commit is contained in:
MarcoFalke
2023-07-04 18:59:49 +02:00
parent f608a409f7
commit fa389d902f
8 changed files with 42 additions and 57 deletions

View File

@@ -19,15 +19,12 @@ FUZZ_TARGET(buffered_file)
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider);
std::optional<BufferedFile> opt_buffered_file;
FILE* fuzzed_file = fuzzed_file_provider.open();
CAutoFile fuzzed_file{fuzzed_file_provider.open(), 0};
try {
opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>());
opt_buffered_file.emplace(fuzzed_file.Get(), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegral<int>());
} catch (const std::ios_base::failure&) {
if (fuzzed_file != nullptr) {
fclose(fuzzed_file);
}
}
if (opt_buffered_file && fuzzed_file != nullptr) {
if (opt_buffered_file && !fuzzed_file.IsNull()) {
bool setpos_fail = false;
LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 10000) {
CallOneOf(

View File

@@ -3,6 +3,7 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <chainparams.h>
#include <clientversion.h>
#include <flatfile.h>
#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>
@@ -27,17 +28,17 @@ FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_fil
{
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
FuzzedFileProvider fuzzed_file_provider = ConsumeFile(fuzzed_data_provider);
FILE* fuzzed_block_file = fuzzed_file_provider.open();
if (fuzzed_block_file == nullptr) {
CAutoFile fuzzed_block_file{fuzzed_file_provider.open(), CLIENT_VERSION};
if (fuzzed_block_file.IsNull()) {
return;
}
if (fuzzed_data_provider.ConsumeBool()) {
// Corresponds to the -reindex case (track orphan blocks across files).
FlatFilePos flat_file_pos;
std::multimap<uint256, FlatFilePos> blocks_with_unknown_parent;
g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file, &flat_file_pos, &blocks_with_unknown_parent);
g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file.Get(), &flat_file_pos, &blocks_with_unknown_parent);
} else {
// Corresponds to the -loadblock= case (orphan blocks aren't tracked across files).
g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file);
g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file.Get());
}
}

View File

@@ -249,18 +249,18 @@ BOOST_AUTO_TEST_CASE(streams_serializedata_xor)
BOOST_AUTO_TEST_CASE(streams_buffered_file)
{
fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
FILE* file = fsbridge::fopen(streams_test_filename, "w+b");
CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
// The value at each offset is the offset.
for (uint8_t j = 0; j < 40; ++j) {
fwrite(&j, 1, 1, file);
file << j;
}
rewind(file);
std::rewind(file.Get());
// The buffer size (second arg) must be greater than the rewind
// amount (third arg).
try {
BufferedFile bfbad{file, 25, 25, 333};
BufferedFile bfbad{file.Get(), 25, 25, 333};
BOOST_CHECK(false);
} catch (const std::exception& e) {
BOOST_CHECK(strstr(e.what(),
@@ -268,7 +268,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
}
// The buffer is 25 bytes, allow rewinding 10 bytes.
BufferedFile bf{file, 25, 10, 333};
BufferedFile bf{file.Get(), 25, 10, 333};
BOOST_CHECK(!bf.eof());
// This member has no functional effect.
@@ -375,7 +375,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
BOOST_CHECK(bf.GetPos() <= 30U);
// We can explicitly close the file, or the destructor will do it.
bf.fclose();
file.fclose();
fs::remove(streams_test_filename);
}
@@ -383,15 +383,15 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
{
fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
FILE* file = fsbridge::fopen(streams_test_filename, "w+b");
CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
// The value at each offset is the byte offset (e.g. byte 1 in the file has the value 0x01).
for (uint8_t j = 0; j < 40; ++j) {
fwrite(&j, 1, 1, file);
file << j;
}
rewind(file);
std::rewind(file.Get());
// The buffer is 25 bytes, allow rewinding 10 bytes.
BufferedFile bf{file, 25, 10, 333};
BufferedFile bf{file.Get(), 25, 10, 333};
uint8_t i;
// This is like bf >> (7-byte-variable), in that it will cause data
@@ -425,7 +425,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
bf.SkipTo(13);
BOOST_CHECK_EQUAL(bf.GetPos(), 13U);
bf.fclose();
file.fclose();
fs::remove(streams_test_filename);
}
@@ -436,16 +436,16 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
fs::path streams_test_filename = m_args.GetDataDirBase() / "streams_test_tmp";
for (int rep = 0; rep < 50; ++rep) {
FILE* file = fsbridge::fopen(streams_test_filename, "w+b");
CAutoFile file{fsbridge::fopen(streams_test_filename, "w+b"), 333};
size_t fileSize = InsecureRandRange(256);
for (uint8_t i = 0; i < fileSize; ++i) {
fwrite(&i, 1, 1, file);
file << i;
}
rewind(file);
std::rewind(file.Get());
size_t bufSize = InsecureRandRange(300) + 1;
size_t rewindSize = InsecureRandRange(bufSize);
BufferedFile bf{file, bufSize, rewindSize, 333};
BufferedFile bf{file.Get(), bufSize, rewindSize, 333};
size_t currentPos = 0;
size_t maxPos = 0;
for (int step = 0; step < 100; ++step) {