diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 889f7b3859b..7602aaba5da 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include namespace { @@ -61,7 +62,6 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data FILE *file = fsbridge::fopen(pathTmp, "wb"); AutoFile fileout{file}; if (fileout.IsNull()) { - fileout.fclose(); remove(pathTmp); LogError("%s: Failed to open file %s\n", __func__, fs::PathToString(pathTmp)); return false; @@ -69,17 +69,22 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data // Serialize if (!SerializeDB(fileout, data)) { - fileout.fclose(); + (void)fileout.fclose(); remove(pathTmp); return false; } if (!fileout.Commit()) { - fileout.fclose(); + (void)fileout.fclose(); remove(pathTmp); LogError("%s: Failed to flush file %s\n", __func__, fs::PathToString(pathTmp)); return false; } - fileout.fclose(); + if (fileout.fclose() != 0) { + const int errno_save{errno}; + remove(pathTmp); + LogError("Failed to close file %s after commit: %s", fs::PathToString(pathTmp), SysErrorString(errno_save)); + return false; + } // replace existing file, if any, with new file if (!RenameOver(pathTmp, path)) { diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp index 004bf8ffc9d..c2bb3c3a310 100644 --- a/src/bench/streams_findbyte.cpp +++ b/src/bench/streams_findbyte.cpp @@ -28,7 +28,7 @@ static void FindByte(benchmark::Bench& bench) }); // Cleanup - file.fclose(); + assert(file.fclose() == 0); fs::remove("streams_tmp"); } diff --git a/src/flatfile.cpp b/src/flatfile.cpp index 388b30efae6..df6596e9405 100644 --- a/src/flatfile.cpp +++ b/src/flatfile.cpp @@ -46,7 +46,9 @@ FILE* FlatFileSeq::Open(const FlatFilePos& pos, bool read_only) const } if (pos.nPos && fseek(file, pos.nPos, SEEK_SET)) { LogPrintf("Unable to seek to position %u of %s\n", pos.nPos, fs::PathToString(path)); - fclose(file); + if (fclose(file) != 0) { + LogError("Unable to close file %s", fs::PathToString(path)); + } return nullptr; } return file; @@ -68,7 +70,10 @@ size_t FlatFileSeq::Allocate(const FlatFilePos& pos, size_t add_size, bool& out_ if (file) { LogDebug(BCLog::VALIDATION, "Pre-allocating up to position 0x%x in %s%05u.dat\n", new_size, m_prefix, pos.nFile); AllocateFileRange(file, pos.nPos, inc_size); - fclose(file); + if (fclose(file) != 0) { + LogError("Cannot close file %s%05u.dat after extending it with %u bytes", m_prefix, pos.nFile, new_size); + return 0; + } return inc_size; } } else { @@ -86,17 +91,24 @@ bool FlatFileSeq::Flush(const FlatFilePos& pos, bool finalize) const return false; } if (finalize && !TruncateFile(file, pos.nPos)) { - fclose(file); LogError("%s: failed to truncate file %d\n", __func__, pos.nFile); + if (fclose(file) != 0) { + LogError("Failed to close file %d", pos.nFile); + } return false; } if (!FileCommit(file)) { - fclose(file); LogError("%s: failed to commit file %d\n", __func__, pos.nFile); + if (fclose(file) != 0) { + LogError("Failed to close file %d", pos.nFile); + } return false; } DirectoryCommit(m_dir); - fclose(file); + if (fclose(file) != 0) { + LogError("Failed to close file %d after flush", pos.nFile); + return false; + } return true; } diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index de43e8e271a..1c1175326a5 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -13,6 +13,7 @@ #include #include #include +#include /* The index database stores three items for each block: the disk location of the encoded filter, * its dSHA256 hash, and the header. Those belonging to blocks on the active chain are indexed by @@ -159,6 +160,11 @@ bool BlockFilterIndex::CustomCommit(CDBBatch& batch) } if (!file.Commit()) { LogError("%s: Failed to commit filter file %d\n", __func__, pos.nFile); + (void)file.fclose(); + return false; + } + if (file.fclose() != 0) { + LogError("Failed to close filter file %d after commit: %s", pos.nFile, SysErrorString(errno)); return false; } @@ -213,6 +219,11 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& } if (!last_file.Commit()) { LogPrintf("%s: Failed to commit filter file %d\n", __func__, pos.nFile); + (void)last_file.fclose(); + return 0; + } + if (last_file.fclose() != 0) { + LogError("Failed to close filter file %d after commit: %s", pos.nFile, SysErrorString(errno)); return 0; } @@ -235,6 +246,12 @@ size_t BlockFilterIndex::WriteFilterToDisk(FlatFilePos& pos, const BlockFilter& } fileout << filter.GetBlockHash() << filter.GetEncodedFilter(); + + if (fileout.fclose() != 0) { + LogError("Failed to close filter file %d: %s", pos.nFile, SysErrorString(errno)); + return 0; + } + return data_size; } diff --git a/src/net.cpp b/src/net.cpp index 217d9a89037..490b766cb1d 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -4005,6 +4005,11 @@ static void CaptureMessageToFile(const CAddress& addr, uint32_t size = data.size(); ser_writedata32(f, size); f << data; + + if (f.fclose() != 0) { + throw std::ios_base::failure( + strprintf("Error closing %s after write, file contents are likely incomplete", fs::PathToString(path))); + } } std::function #include #include +#include #include #include @@ -941,13 +942,13 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt return false; } + // Open history file to append + AutoFile file{OpenUndoFile(pos)}; + if (file.IsNull()) { + LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString()); + return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); + } { - // Open history file to append - AutoFile file{OpenUndoFile(pos)}; - if (file.IsNull()) { - LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString()); - return FatalError(m_opts.notifications, state, _("Failed to write undo data.")); - } BufferedWriter fileout{file}; // Write index header @@ -960,8 +961,13 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt // Write undo data & checksum fileout << blockundo << hasher.GetHash(); } + // BufferedWriter will flush pending data to file when fileout goes out of scope. + } - fileout.flush(); // Make sure `AutoFile`/`BufferedWriter` go out of scope before we call `FlushUndoFile` + // Make sure that the file is closed before we call `FlushUndoFile`. + if (file.fclose() != 0) { + LogError("Failed to close block undo file %s: %s", pos.ToString(), SysErrorString(errno)); + return FatalError(m_opts.notifications, state, _("Failed to close block undo file.")); } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) @@ -1094,13 +1100,22 @@ FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) m_opts.notifications.fatalError(_("Failed to write block.")); return FlatFilePos(); } - BufferedWriter fileout{file}; + { + BufferedWriter fileout{file}; + + // Write index header + fileout << GetParams().MessageStart() << block_size; + pos.nPos += STORAGE_HEADER_BYTES; + // Write block + fileout << TX_WITH_WITNESS(block); + } + + if (file.fclose() != 0) { + LogError("Failed to close block file %s: %s", pos.ToString(), SysErrorString(errno)); + m_opts.notifications.fatalError(_("Failed to close file when writing block.")); + return FlatFilePos(); + } - // Write index header - fileout << GetParams().MessageStart() << block_size; - pos.nPos += STORAGE_HEADER_BYTES; - // Write block - fileout << TX_WITH_WITNESS(block); return pos; } @@ -1143,6 +1158,11 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts) #endif )}; xor_key_file << xor_key; + if (xor_key_file.fclose() != 0) { + throw std::runtime_error{strprintf("Error closing XOR key file %s: %s", + fs::PathToString(xor_key_path), + SysErrorString(errno))}; + } } // If the user disabled the key, it must be zero. if (!opts.use_xor && xor_key != decltype(xor_key){}) { diff --git a/src/node/mempool_persist.cpp b/src/node/mempool_persist.cpp index ff7de8c64a2..d4d9263973e 100644 --- a/src/node/mempool_persist.cpp +++ b/src/node/mempool_persist.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include @@ -168,7 +169,8 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock auto mid = SteadyClock::now(); - AutoFile file{mockable_fopen_function(dump_path + ".new", "wb")}; + const fs::path file_fspath{dump_path + ".new"}; + AutoFile file{mockable_fopen_function(file_fspath, "wb")}; if (file.IsNull()) { return false; } @@ -199,9 +201,14 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock LogInfo("Writing %d unbroadcast transactions to file.\n", unbroadcast_txids.size()); file << unbroadcast_txids; - if (!skip_file_commit && !file.Commit()) + if (!skip_file_commit && !file.Commit()) { + (void)file.fclose(); throw std::runtime_error("Commit failed"); - file.fclose(); + } + if (file.fclose() != 0) { + throw std::runtime_error( + strprintf("Error closing %s: %s", fs::PathToString(file_fspath), SysErrorString(errno))); + } if (!RenameOver(dump_path + ".new", dump_path)) { throw std::runtime_error("Rename failed"); } @@ -213,6 +220,7 @@ bool DumpMempool(const CTxMemPool& pool, const fs::path& dump_path, FopenFn mock fs::file_size(dump_path)); } catch (const std::exception& e) { LogInfo("Failed to dump mempool: %s. Continuing anyway.\n", e.what()); + (void)file.fclose(); return false; } return true; diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp index 95fe2aff970..feb91662296 100644 --- a/src/policy/fees.cpp +++ b/src/policy/fees.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -963,9 +964,14 @@ void CBlockPolicyEstimator::FlushFeeEstimates() AutoFile est_file{fsbridge::fopen(m_estimation_filepath, "wb")}; if (est_file.IsNull() || !Write(est_file)) { LogPrintf("Failed to write fee estimates to %s. Continue anyway.\n", fs::PathToString(m_estimation_filepath)); - } else { - LogPrintf("Flushed fee estimates to %s.\n", fs::PathToString(m_estimation_filepath.filename())); + (void)est_file.fclose(); + return; } + if (est_file.fclose() != 0) { + LogError("Failed to close fee estimates file %s: %s. Continuing anyway.", fs::PathToString(m_estimation_filepath), SysErrorString(errno)); + return; + } + LogPrintf("Flushed fee estimates to %s.\n", fs::PathToString(m_estimation_filepath.filename())); } bool CBlockPolicyEstimator::Write(AutoFile& fileout) const diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 24b45a816df..ca872cf6ac8 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -47,6 +47,7 @@ #include #include #include +#include #include #include #include @@ -82,7 +83,7 @@ UniValue WriteUTXOSnapshot( CCoinsViewCursor* pcursor, CCoinsStats* maybe_stats, const CBlockIndex* tip, - AutoFile& afile, + AutoFile&& afile, const fs::path& path, const fs::path& temppath, const std::function& interruption_point = {}); @@ -3114,7 +3115,14 @@ static RPCHelpMan dumptxoutset() } } - UniValue result = WriteUTXOSnapshot(*chainstate, cursor.get(), &stats, tip, afile, path, temppath, node.rpc_interruption_point); + UniValue result = WriteUTXOSnapshot(*chainstate, + cursor.get(), + &stats, + tip, + std::move(afile), + path, + temppath, + node.rpc_interruption_point); fs::rename(temppath, path); result.pushKV("path", path.utf8string()); @@ -3166,7 +3174,7 @@ UniValue WriteUTXOSnapshot( CCoinsViewCursor* pcursor, CCoinsStats* maybe_stats, const CBlockIndex* tip, - AutoFile& afile, + AutoFile&& afile, const fs::path& path, const fs::path& temppath, const std::function& interruption_point) @@ -3225,7 +3233,10 @@ UniValue WriteUTXOSnapshot( CHECK_NONFATAL(written_coins_count == maybe_stats->coins_count); - afile.fclose(); + if (afile.fclose() != 0) { + throw std::ios_base::failure( + strprintf("Error closing %s: %s", fs::PathToString(temppath), SysErrorString(errno))); + } UniValue result(UniValue::VOBJ); result.pushKV("coins_written", written_coins_count); @@ -3240,12 +3251,19 @@ UniValue WriteUTXOSnapshot( UniValue CreateUTXOSnapshot( node::NodeContext& node, Chainstate& chainstate, - AutoFile& afile, + AutoFile&& afile, const fs::path& path, const fs::path& tmppath) { auto [cursor, stats, tip]{WITH_LOCK(::cs_main, return PrepareUTXOSnapshot(chainstate, node.rpc_interruption_point))}; - return WriteUTXOSnapshot(chainstate, cursor.get(), &stats, tip, afile, path, tmppath, node.rpc_interruption_point); + return WriteUTXOSnapshot(chainstate, + cursor.get(), + &stats, + tip, + std::move(afile), + path, + tmppath, + node.rpc_interruption_point); } static RPCHelpMan loadtxoutset() diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index b42fe96fd30..0e42bed9479 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -51,7 +51,7 @@ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], UniValue CreateUTXOSnapshot( node::NodeContext& node, Chainstate& chainstate, - AutoFile& afile, + AutoFile&& afile, const fs::path& path, const fs::path& tmppath); diff --git a/src/streams.cpp b/src/streams.cpp index 19c2b474452..2c873e50d3b 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -85,6 +85,7 @@ void AutoFile::write(std::span src) if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { throw std::ios_base::failure("AutoFile::write: write failed"); } + m_was_written = true; if (m_position.has_value()) *m_position += src.size(); } else { std::array buf; @@ -107,6 +108,7 @@ void AutoFile::write_buffer(std::span src) if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { throw std::ios_base::failure("AutoFile::write_buffer: write failed"); } + m_was_written = true; if (m_position) *m_position += src.size(); } @@ -117,6 +119,7 @@ bool AutoFile::Commit() bool AutoFile::Truncate(unsigned size) { + m_was_written = true; return ::TruncateFile(m_file, size); } diff --git a/src/streams.h b/src/streams.h index ebef958f544..ae9cb94bae3 100644 --- a/src/streams.h +++ b/src/streams.h @@ -6,10 +6,13 @@ #ifndef BITCOIN_STREAMS_H #define BITCOIN_STREAMS_H +#include #include #include #include +#include #include +#include #include #include @@ -387,7 +390,13 @@ public: * * Will automatically close the file when it goes out of scope if not null. * If you're returning the file pointer, return file.release(). - * If you need to close the file early, use file.fclose() instead of fclose(file). + * If you need to close the file early, use autofile.fclose() instead of fclose(underlying_FILE). + * + * @note If the file has been written to, then the caller must close it + * explicitly with the `fclose()` method, check if it returns an error and treat + * such an error as if the `write()` method failed. The OS's `fclose(3)` may + * fail to flush to disk data that has been previously written, rendering the + * file corrupt. */ class AutoFile { @@ -395,11 +404,28 @@ protected: std::FILE* m_file; std::vector m_xor; std::optional m_position; + bool m_was_written{false}; public: explicit AutoFile(std::FILE* file, std::vector data_xor={}); - ~AutoFile() { fclose(); } + ~AutoFile() + { + if (m_was_written) { + // Callers that wrote to the file must have closed it explicitly + // with the fclose() method and checked that the close succeeded. + // This is because here in the destructor we have no way to signal + // errors from fclose() which, after write, could mean the file is + // corrupted and must be handled properly at the call site. + // Destructors in C++ cannot signal an error to the callers because + // they do not return a value and are not allowed to throw exceptions. + Assume(IsNull()); + } + + if (fclose() != 0) { + LogError("Failed to close file: %s", SysErrorString(errno)); + } + } // Disallow copies AutoFile(const AutoFile&) = delete; @@ -407,7 +433,7 @@ public: bool feof() const { return std::feof(m_file); } - int fclose() + [[nodiscard]] int fclose() { if (auto rel{release()}) return std::fclose(rel); return 0; diff --git a/src/test/flatfile_tests.cpp b/src/test/flatfile_tests.cpp index 21a36d9d434..d94cab640b8 100644 --- a/src/test/flatfile_tests.cpp +++ b/src/test/flatfile_tests.cpp @@ -46,6 +46,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) { AutoFile file{seq.Open(FlatFilePos(0, pos1))}; file << LIMITED_STRING(line1, 256); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); } // Attempt to append to file opened in read-only mode. @@ -58,6 +59,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) { AutoFile file{seq.Open(FlatFilePos(0, pos2))}; file << LIMITED_STRING(line2, 256); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); } // Read text from file in read-only mode. @@ -79,6 +81,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) file >> LIMITED_STRING(text, 256); BOOST_CHECK_EQUAL(text, line2); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); } // Ensure another file in the sequence has no data. @@ -86,6 +89,7 @@ BOOST_AUTO_TEST_CASE(flatfile_open) std::string text; AutoFile file{seq.Open(FlatFilePos(1, pos2))}; BOOST_CHECK_THROW(file >> LIMITED_STRING(text, 256), std::ios_base::failure); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); } } diff --git a/src/test/fuzz/autofile.cpp b/src/test/fuzz/autofile.cpp index 81761c7bf96..8d17624da66 100644 --- a/src/test/fuzz/autofile.cpp +++ b/src/test/fuzz/autofile.cpp @@ -47,7 +47,7 @@ FUZZ_TARGET(autofile) } }, [&] { - auto_file.fclose(); + (void)auto_file.fclose(); }, [&] { ReadFromStream(fuzzed_data_provider, auto_file); @@ -62,5 +62,7 @@ FUZZ_TARGET(autofile) if (f != nullptr) { fclose(f); } + } else { + (void)auto_file.fclose(); } } diff --git a/src/test/fuzz/policy_estimator.cpp b/src/test/fuzz/policy_estimator.cpp index cda7725a136..37e37964182 100644 --- a/src/test/fuzz/policy_estimator.cpp +++ b/src/test/fuzz/policy_estimator.cpp @@ -111,5 +111,6 @@ FUZZ_TARGET(policy_estimator, .init = initialize_policy_estimator) AutoFile fuzzed_auto_file{fuzzed_file_provider.open()}; block_policy_estimator.Write(fuzzed_auto_file); block_policy_estimator.Read(fuzzed_auto_file); + (void)fuzzed_auto_file.fclose(); } } diff --git a/src/test/fuzz/policy_estimator_io.cpp b/src/test/fuzz/policy_estimator_io.cpp index 02dc5fec867..36a1b5f89d8 100644 --- a/src/test/fuzz/policy_estimator_io.cpp +++ b/src/test/fuzz/policy_estimator_io.cpp @@ -32,4 +32,5 @@ FUZZ_TARGET(policy_estimator_io, .init = initialize_policy_estimator_io) if (block_policy_estimator.Read(fuzzed_auto_file)) { block_policy_estimator.Write(fuzzed_auto_file); } + (void)fuzzed_auto_file.fclose(); } diff --git a/src/test/fuzz/utxo_snapshot.cpp b/src/test/fuzz/utxo_snapshot.cpp index 16196b0e25f..0916c6258c7 100644 --- a/src/test/fuzz/utxo_snapshot.cpp +++ b/src/test/fuzz/utxo_snapshot.cpp @@ -150,6 +150,7 @@ void utxo_snapshot_fuzz(FuzzBufferType buffer) WriteCompactSize(outfile, 999); // index of coin outfile << Coin{coinbase->vout[0], /*nHeightIn=*/999, /*fCoinBaseIn=*/0}; } + assert(outfile.fclose() == 0); } const auto ActivateFuzzedSnapshot{[&] { diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index c7b5cd353e0..9e8281d26a7 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -39,6 +39,7 @@ BOOST_AUTO_TEST_CASE(xor_file) #endif AutoFile xor_file{raw_file(mode), xor_pat}; xor_file << test1 << test2; + BOOST_REQUIRE_EQUAL(xor_file.fclose(), 0); } { // Read raw from disk @@ -379,8 +380,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file) // by the rewind window (relative to our farthest read position, 40). BOOST_CHECK(bf.GetPos() <= 30U); - // We can explicitly close the file, or the destructor will do it. - file.fclose(); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); fs::remove(streams_test_filename); } @@ -430,7 +430,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip) bf.SkipTo(13); BOOST_CHECK_EQUAL(bf.GetPos(), 13U); - file.fclose(); + BOOST_REQUIRE_EQUAL(file.fclose(), 0); fs::remove(streams_test_filename); } @@ -551,6 +551,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) if (maxPos < currentPos) maxPos = currentPos; } + BOOST_REQUIRE_EQUAL(file.fclose(), 0); } fs::remove(streams_test_filename); } @@ -566,7 +567,9 @@ BOOST_AUTO_TEST_CASE(buffered_reader_matches_autofile_random_content) // Write out the file with random content { - AutoFile{test_file.Open(pos, /*read_only=*/false), obfuscation}.write(m_rng.randbytes(file_size)); + AutoFile f{test_file.Open(pos, /*read_only=*/false), obfuscation}; + f.write(m_rng.randbytes(file_size)); + BOOST_REQUIRE_EQUAL(f.fclose(), 0); } BOOST_CHECK_EQUAL(fs::file_size(test_file.FileName(pos)), file_size); @@ -623,18 +626,21 @@ BOOST_AUTO_TEST_CASE(buffered_writer_matches_autofile_random_content) AutoFile direct_file{test_direct.Open(pos, /*read_only=*/false), obfuscation}; AutoFile buffered_file{test_buffered.Open(pos, /*read_only=*/false), obfuscation}; - BufferedWriter buffered{buffered_file, buf_size}; + { + BufferedWriter buffered{buffered_file, buf_size}; - for (size_t total_written{0}; total_written < file_size;) { - const size_t write_size{Assert(std::min(1 + m_rng.randrange(m_rng.randbool() ? buf_size : 2 * buf_size), file_size - total_written))}; + for (size_t total_written{0}; total_written < file_size;) { + const size_t write_size{Assert(std::min(1 + m_rng.randrange(m_rng.randbool() ? buf_size : 2 * buf_size), file_size - total_written))}; - auto current_span = std::span{test_data}.subspan(total_written, write_size); - direct_file.write(current_span); - buffered.write(current_span); + auto current_span = std::span{test_data}.subspan(total_written, write_size); + direct_file.write(current_span); + buffered.write(current_span); - total_written += write_size; + total_written += write_size; + } } - // Destructors of AutoFile and BufferedWriter will flush/close here + BOOST_REQUIRE_EQUAL(buffered_file.fclose(), 0); + BOOST_REQUIRE_EQUAL(direct_file.fclose(), 0); } // Compare the resulting files @@ -671,12 +677,14 @@ BOOST_AUTO_TEST_CASE(buffered_writer_reader) const fs::path test_file{m_args.GetDataDirBase() / "test_buffered_write_read.bin"}; // Write out the values through a precisely sized BufferedWriter + AutoFile file{fsbridge::fopen(test_file, "w+b")}; { - AutoFile file{fsbridge::fopen(test_file, "w+b")}; BufferedWriter f(file, sizeof(v1) + sizeof(v2) + sizeof(v3)); f << v1 << v2; f.write(std::as_bytes(std::span{&v3, 1})); } + BOOST_REQUIRE_EQUAL(file.fclose(), 0); + // Read back and verify using BufferedReader { uint32_t _v1{0}, _v2{0}, _v3{0}; diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index e604b44c16a..d5100d15b16 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -47,8 +47,11 @@ CreateAndActivateUTXOSnapshot( FILE* outfile{fsbridge::fopen(snapshot_path, "wb")}; AutoFile auto_outfile{outfile}; - UniValue result = CreateUTXOSnapshot( - node, node.chainman->ActiveChainstate(), auto_outfile, snapshot_path, snapshot_path); + UniValue result = CreateUTXOSnapshot(node, + node.chainman->ActiveChainstate(), + std::move(auto_outfile), // Will close auto_outfile. + snapshot_path, + snapshot_path); LogPrintf( "Wrote UTXO snapshot to %s: %s\n", fs::PathToString(snapshot_path.make_preferred()), result.write()); diff --git a/src/wallet/test/fuzz/wallet_bdb_parser.cpp b/src/wallet/test/fuzz/wallet_bdb_parser.cpp index ba644076077..f15548d4d6c 100644 --- a/src/wallet/test/fuzz/wallet_bdb_parser.cpp +++ b/src/wallet/test/fuzz/wallet_bdb_parser.cpp @@ -36,6 +36,7 @@ FUZZ_TARGET(wallet_bdb_parser, .init = initialize_wallet_bdb_parser) { AutoFile outfile{fsbridge::fopen(wallet_path, "wb")}; outfile << std::span{buffer}; + assert(outfile.fclose() == 0); } const DatabaseOptions options{};