Merge bitcoin/bitcoin#28904: Drop CAutoFile

src/bench/load_external.cpp

@@ -54,7 +54,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
     bench.run([&] {
         // "rb" is "binary, O_RDONLY", positioned to the start of the file.
         // The file will be closed by LoadExternalBlockFile().
-        FILE* file{fsbridge::fopen(blkfile, "rb")};
+        AutoFile file{fsbridge::fopen(blkfile, "rb")};
         chainstate.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
     });
     fs::remove(blkfile);

src/index/txindex.cpp

@@ -80,7 +80,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
         return false;
     }
 
-    CAutoFile file(OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION);
+    AutoFile file{OpenBlockFile(postx, true)};
     if (file.IsNull()) {
         return error("%s: OpenBlockFile failed", __func__);
     }

src/node/blockstorage.cpp

@@ -379,7 +379,7 @@ bool BlockManager::LoadBlockIndexDB()
     }
     for (std::set<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {
         FlatFilePos pos(*it, 0);
-        if (CAutoFile(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION).IsNull()) {
+        if (AutoFile{OpenBlockFile(pos, true)}.IsNull()) {
             return false;
         }
     }
@@ -487,13 +487,13 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)
 static bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock, const CMessageHeader::MessageStartChars& messageStart)
 {
     // Open history file to append
-    CAutoFile fileout(OpenUndoFile(pos), SER_DISK, CLIENT_VERSION);
+    AutoFile fileout{OpenUndoFile(pos)};
     if (fileout.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
 
     // Write index header
-    unsigned int nSize = GetSerializeSize(blockundo, fileout.GetVersion());
+    unsigned int nSize = GetSerializeSize(blockundo, CLIENT_VERSION);
     fileout << messageStart << nSize;
 
     // Write undo data
@@ -522,14 +522,14 @@ bool UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex* pindex)
     }
 
     // Open history file to read
-    CAutoFile filein(OpenUndoFile(pos, true), SER_DISK, CLIENT_VERSION);
+    AutoFile filein{OpenUndoFile(pos, true)};
     if (filein.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
 
     // Read block
     uint256 hashChecksum;
-    CHashVerifier<CAutoFile> verifier(&filein); // We need a CHashVerifier as reserializing may lose data
+    CHashVerifier<AutoFile> verifier(&filein); // We need a CHashVerifier as reserializing may lose data
     try {
         verifier << pindex->pprev->GetBlockHash();
         verifier >> blockundo;
@@ -597,15 +597,15 @@ static FlatFileSeq UndoFileSeq()
     return FlatFileSeq(gArgs.GetBlocksDirPath(), "rev", UNDOFILE_CHUNK_SIZE);
 }
 
-FILE* OpenBlockFile(const FlatFilePos& pos, bool fReadOnly)
+AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly)
 {
-    return BlockFileSeq().Open(pos, fReadOnly);
+    return AutoFile{BlockFileSeq().Open(pos, fReadOnly)};
 }
 
 /** Open an undo file (rev?????.dat) */
-static FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly)
+static AutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly)
 {
-    return UndoFileSeq().Open(pos, fReadOnly);
+    return AutoFile{UndoFileSeq().Open(pos, fReadOnly)};
 }
 
 fs::path GetBlockPosFilename(const FlatFilePos& pos)
@@ -693,13 +693,13 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP
 static bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos, const CMessageHeader::MessageStartChars& messageStart)
 {
     // Open history file to append
-    CAutoFile fileout(OpenBlockFile(pos), SER_DISK, CLIENT_VERSION);
+    AutoFile fileout{OpenBlockFile(pos)};
     if (fileout.IsNull()) {
         return error("WriteBlockToDisk: OpenBlockFile failed");
     }
 
     // Write index header
-    unsigned int nSize = GetSerializeSize(block, fileout.GetVersion());
+    unsigned int nSize = GetSerializeSize(block, CLIENT_VERSION);
     fileout << messageStart << nSize;
 
     // Write block
@@ -748,7 +748,7 @@ bool ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos, const Consensus::P
     block.SetNull();
 
     // Open history file to read
-    CAutoFile filein(OpenBlockFile(pos, true), SER_DISK, CLIENT_VERSION);
+    AutoFile filein{OpenBlockFile(pos, true)};
     if (filein.IsNull()) {
         return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());
     }
@@ -840,8 +840,8 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile
                 if (!fs::exists(GetBlockPosFilename(pos))) {
                     break; // No block files left to reindex
                 }
-                FILE* file = OpenBlockFile(pos, true);
-                if (!file) {
+                AutoFile file{OpenBlockFile(pos, true)};
+                if (file.IsNull()) {
                     break; // This error is logged in OpenBlockFile
                 }
                 LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile);
@@ -861,8 +861,8 @@ void ThreadImport(ChainstateManager& chainman, std::vector<fs::path> vImportFile
 
         // -loadblock=
         for (const fs::path& path : vImportFiles) {
-            FILE *file = fsbridge::fopen(path, "rb");
-            if (file) {
+            AutoFile file{fsbridge::fopen(path, "rb")};
+            if (!file.IsNull()) {
                 LogPrintf("Importing blocks file %s...\n", fs::PathToString(path));
                 chainman.ActiveChainstate().LoadExternalBlockFile(file);
                 if (ShutdownRequested()) {

src/node/blockstorage.h

@@ -19,6 +19,7 @@
 extern RecursiveMutex cs_main;
 
 class ArgsManager;
+class AutoFile;
 class BlockValidationState;
 class CBlock;
 class CBlockUndo;
@@ -211,7 +212,7 @@ class BlockManager
 void CleanupBlockRevFiles();
 
 /** Open a block file (blk?????.dat) */
-FILE* OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false);
+AutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false);
 /** Translation to a filesystem path */
 fs::path GetBlockPosFilename(const FlatFilePos& pos);
 

src/streams.h

@@ -596,6 +596,9 @@ class CAutoFile
     }
 };
 
+/** Alias for CAutoFile to match Bitcoin's naming convention */
+using AutoFile = CAutoFile;
+
 /** Non-refcounted RAII wrapper around a FILE* that implements a ring buffer to
  *  deserialize from. It guarantees the ability to rewind a given number of bytes.
  *

src/test/fuzz/buffered_file.cpp

@@ -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<CBufferedFile> opt_buffered_file;
-    FILE* fuzzed_file = fuzzed_file_provider.open();
+    AutoFile fuzzed_file{fuzzed_file_provider.open()};
     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>(), 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>(), 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(
@@ -63,6 +60,5 @@ FUZZ_TARGET(buffered_file)
         }
         opt_buffered_file->GetPos();
         opt_buffered_file->GetType();
-        opt_buffered_file->GetVersion();
     }
 }

src/test/fuzz/load_external_block_file.cpp

@@ -27,8 +27,8 @@ 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) {
+    AutoFile fuzzed_block_file{fuzzed_file_provider.open()};
+    if (fuzzed_block_file.IsNull()) {
         return;
     }
     if (fuzzed_data_provider.ConsumeBool()) {

src/test/streams_tests.cpp

@@ -198,32 +198,28 @@ 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");
+    AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
 
     // 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);
+    file.rewind();
 
     // The buffer size (second arg) must be greater than the rewind
     // amount (third arg).
     try {
-        CBufferedFile bfbad(file, 25, 25, 222, 333);
+        CBufferedFile bfbad(file.Get(), 25, 25, 222, 333);
         BOOST_CHECK(false);
     } catch (const std::exception& e) {
         BOOST_CHECK(strstr(e.what(),
                         "Rewind limit must be less than buffer size") != nullptr);
     }
 
     // The buffer is 25 bytes, allow rewinding 10 bytes.
-    CBufferedFile bf(file, 25, 10, 222, 333);
+    CBufferedFile bf(file.Get(), 25, 10, 222, 333);
     BOOST_CHECK(!bf.eof());
 
-    // These two members have no functional effect.
-    BOOST_CHECK_EQUAL(bf.GetType(), 222);
-    BOOST_CHECK_EQUAL(bf.GetVersion(), 333);
-
     uint8_t i;
     bf >> i;
     BOOST_CHECK_EQUAL(i, 0);
@@ -333,15 +329,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");
+    AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
     // 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);
+    file.rewind();
 
     // The buffer is 25 bytes, allow rewinding 10 bytes.
-    CBufferedFile bf(file, 25, 10, 222, 333);
+    CBufferedFile bf(file.Get(), 25, 10, 222, 333);
 
     uint8_t i;
     // This is like bf >> (7-byte-variable), in that it will cause data
@@ -386,16 +382,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");
+        AutoFile file{fsbridge::fopen(streams_test_filename, "w+b")};
         size_t fileSize = InsecureRandRange(256);
         for (uint8_t i = 0; i < fileSize; ++i) {
-            fwrite(&i, 1, 1, file);
+            file << i;
         }
-        rewind(file);
+        file.rewind();
 
         size_t bufSize = InsecureRandRange(300) + 1;
         size_t rewindSize = InsecureRandRange(bufSize);
-        CBufferedFile bf(file, bufSize, rewindSize, 222, 333);
+        CBufferedFile bf(file.Get(), bufSize, rewindSize, 222, 333);
         size_t currentPos = 0;
         size_t maxPos = 0;
         for (int step = 0; step < 100; ++step) {

src/validation.cpp

@@ -4940,7 +4940,7 @@ bool CChainState::LoadGenesisBlock()
 }
 
 void CChainState::LoadExternalBlockFile(
-    FILE* fileIn,
+    AutoFile& fileIn,
     FlatFilePos* dbp,
     std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent)
 {
@@ -4955,7 +4955,7 @@ void CChainState::LoadExternalBlockFile(
     try {
         unsigned int nMaxBlockSize = MaxBlockSize();
         // This takes over fileIn and calls fclose() on it in the CBufferedFile destructor
-        CBufferedFile blkdat(fileIn, 2*nMaxBlockSize, nMaxBlockSize+8, SER_DISK, CLIENT_VERSION);
+        CBufferedFile blkdat(fileIn.Get(), 2*nMaxBlockSize, nMaxBlockSize+8, SER_DISK, CLIENT_VERSION);
         // nRewind indicates where to resume scanning in case something goes wrong,
         // such as a block fails to deserialize.
         uint64_t nRewind = blkdat.GetPos();

src/validation.h

@@ -42,6 +42,7 @@
 #include <utility>
 #include <vector>
 
+class AutoFile;
 class CChainState;
 class CBlockTreeDB;
 class CChainParams;
@@ -634,7 +635,7 @@ class CChainState
      *                                              (only used for reindex)
      * */
     void LoadExternalBlockFile(
-        FILE* fileIn,
+        AutoFile& fileIn,
         FlatFilePos* dbp = nullptr,
         std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr)
         EXCLUSIVE_LOCKS_REQUIRED(!m_chainstate_mutex);