Make BufferedFile to be a CAutoFile wrapper

This refactor allows to forward some calls to the underlying CAutoFile,
instead of re-implementing the logic in the buffered file.

Author: MarcoFalke

src/bench/load_external.cpp

@@ -56,7 +56,7 @@ static void LoadExternalBlockFile(benchmark::Bench& bench)
         // "rb" is "binary, O_RDONLY", positioned to the start of the file.
         // The file will be closed by LoadExternalBlockFile().
         CAutoFile file{fsbridge::fopen(blkfile, "rb"), CLIENT_VERSION};
-        testing_setup->m_node.chainman->LoadExternalBlockFile(file.Get(), &pos, &blocks_with_unknown_parent);
+        testing_setup->m_node.chainman->LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
     });
     fs::remove(blkfile);
 }

src/bench/streams_findbyte.cpp

@@ -20,7 +20,7 @@ static void FindByte(benchmark::Bench& bench)
     data[file_size-1] = 1;
     file << data;
     std::rewind(file.Get());
-    BufferedFile bf{file.Get(), /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0};
+    BufferedFile bf{file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size};
 
     bench.run([&] {
         bf.SetPos(0);

src/node/blockstorage.cpp

@@ -1020,7 +1020,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
                     break; // This error is logged in OpenBlockFile
                 }
                 LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile);
-                chainman.LoadExternalBlockFile(file.Get(), &pos, &blocks_with_unknown_parent);
+                chainman.LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent);
                 if (chainman.m_interrupt) {
                     LogPrintf("Interrupt requested. Exit %s\n", __func__);
                     return;
@@ -1039,7 +1039,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
             CAutoFile file{fsbridge::fopen(path, "rb"), CLIENT_VERSION};
             if (!file.IsNull()) {
                 LogPrintf("Importing blocks file %s...\n", fs::PathToString(path));
-                chainman.LoadExternalBlockFile(file.Get());
+                chainman.LoadExternalBlockFile(file);
                 if (chainman.m_interrupt) {
                     LogPrintf("Interrupt requested. Exit %s\n", __func__);
                     return;

src/streams.h

@@ -571,7 +571,7 @@ class CAutoFile : public AutoFile
     }
 };
 
-/** Non-refcounted RAII wrapper around a FILE* that implements a ring buffer to
+/** Wrapper around a CAutoFile& that implements a ring buffer to
  *  deserialize from. It guarantees the ability to rewind a given number of bytes.
  *
  *  Will automatically close the file when it goes out of scope if not null.
@@ -580,9 +580,7 @@ class CAutoFile : public AutoFile
 class BufferedFile
 {
 private:
-    const int nVersion;
-
-    FILE *src;            //!< source file
+    CAutoFile& m_src;
     uint64_t nSrcPos{0};  //!< how many bytes have been read from source
     uint64_t m_read_pos{0}; //!< how many bytes have been read from this
     uint64_t nReadLimit;  //!< up to which position we're allowed to read
@@ -598,9 +596,9 @@ class BufferedFile
             readNow = nAvail;
         if (readNow == 0)
             return false;
-        size_t nBytes = fread((void*)&vchBuf[pos], 1, readNow, src);
+        size_t nBytes{m_src.detail_fread(Span{vchBuf}.subspan(pos, readNow))};
         if (nBytes == 0) {
-            throw std::ios_base::failure(feof(src) ? "BufferedFile::Fill: end of file" : "BufferedFile::Fill: fread failed");
+            throw std::ios_base::failure{m_src.feof() ? "BufferedFile::Fill: end of file" : "BufferedFile::Fill: fread failed"};
         }
         nSrcPos += nBytes;
         return true;
@@ -629,19 +627,18 @@ class BufferedFile
     }
 
 public:
-    BufferedFile(FILE* fileIn, uint64_t nBufSize, uint64_t nRewindIn, int nVersionIn)
-        : nVersion{nVersionIn}, nReadLimit{std::numeric_limits<uint64_t>::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0})
+    BufferedFile(CAutoFile& file, uint64_t nBufSize, uint64_t nRewindIn)
+        : m_src{file}, nReadLimit{std::numeric_limits<uint64_t>::max()}, nRewind{nRewindIn}, vchBuf(nBufSize, std::byte{0})
     {
         if (nRewindIn >= nBufSize)
             throw std::ios_base::failure("Rewind limit must be less than buffer size");
-        src = fileIn;
     }
 
-    int GetVersion() const { return nVersion; }
+    int GetVersion() const { return m_src.GetVersion(); }
 
     //! check whether we're at the end of the source file
     bool eof() const {
-        return m_read_pos == nSrcPos && feof(src);
+        return m_read_pos == nSrcPos && m_src.feof();
     }
 
     //! read a number of bytes

src/test/fuzz/buffered_file.cpp

@@ -21,7 +21,7 @@ FUZZ_TARGET(buffered_file)
     std::optional<BufferedFile> opt_buffered_file;
     CAutoFile fuzzed_file{fuzzed_file_provider.open(), 0};
     try {
-        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>());
+        opt_buffered_file.emplace(fuzzed_file, fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096), fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096));
     } catch (const std::ios_base::failure&) {
     }
     if (opt_buffered_file && !fuzzed_file.IsNull()) {

src/test/fuzz/load_external_block_file.cpp

@@ -36,9 +36,9 @@ FUZZ_TARGET(load_external_block_file, .init = initialize_load_external_block_fil
         // 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.Get(), &flat_file_pos, &blocks_with_unknown_parent);
+        g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file, &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.Get());
+        g_setup->m_node.chainman->LoadExternalBlockFile(fuzzed_block_file);
     }
 }

src/test/streams_tests.cpp

@@ -260,15 +260,15 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file)
     // The buffer size (second arg) must be greater than the rewind
     // amount (third arg).
     try {
-        BufferedFile bfbad{file.Get(), 25, 25, 333};
+        BufferedFile bfbad{file, 25, 25};
         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.
-    BufferedFile bf{file.Get(), 25, 10, 333};
+    BufferedFile bf{file, 25, 10};
     BOOST_CHECK(!bf.eof());
 
     // This member has no functional effect.
@@ -391,7 +391,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_skip)
     std::rewind(file.Get());
 
     // The buffer is 25 bytes, allow rewinding 10 bytes.
-    BufferedFile bf{file.Get(), 25, 10, 333};
+    BufferedFile bf{file, 25, 10};
 
     uint8_t i;
     // This is like bf >> (7-byte-variable), in that it will cause data
@@ -445,7 +445,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand)
 
         size_t bufSize = InsecureRandRange(300) + 1;
         size_t rewindSize = InsecureRandRange(bufSize);
-        BufferedFile bf{file.Get(), bufSize, rewindSize, 333};
+        BufferedFile bf{file, bufSize, rewindSize};
         size_t currentPos = 0;
         size_t maxPos = 0;
         for (int step = 0; step < 100; ++step) {

src/validation.cpp

@@ -4577,7 +4577,7 @@ bool Chainstate::LoadGenesisBlock()
 }
 
 void ChainstateManager::LoadExternalBlockFile(
-    FILE* fileIn,
+    CAutoFile& file_in,
     FlatFilePos* dbp,
     std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent)
 {
@@ -4589,7 +4589,7 @@ void ChainstateManager::LoadExternalBlockFile(
 
     int nLoaded = 0;
     try {
-        BufferedFile blkdat{fileIn, 2 * MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE + 8, CLIENT_VERSION};
+        BufferedFile blkdat{file_in, 2 * MAX_BLOCK_SERIALIZED_SIZE, MAX_BLOCK_SERIALIZED_SIZE + 8};
         // 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

@@ -607,7 +607,6 @@ class Chainstate
     bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size)
         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
-
     /**
      * Update the on-disk chain state.
      * The caches and indexes are flushed depending on the mode we're called with
@@ -1087,14 +1086,14 @@ class ChainstateManager
      * -loadblock= option. There's no unknown-parent tracking, so the last two arguments are omitted.
      *
      *
-     * @param[in]     fileIn                        FILE handle to file containing blocks to read
+     * @param[in]     file_in                       File containing blocks to read
      * @param[in]     dbp                           (optional) Disk block position (only for reindex)
      * @param[in,out] blocks_with_unknown_parent    (optional) Map of disk positions for blocks with
      *                                              unknown parent, key is parent block hash
      *                                              (only used for reindex)
      * */
     void LoadExternalBlockFile(
-        FILE* fileIn,
+        CAutoFile& file_in,
         FlatFilePos* dbp = nullptr,
         std::multimap<uint256, FlatFilePos>* blocks_with_unknown_parent = nullptr);