Return CAutoFile from BlockManager::Open*File()

MarcoFalke · MarcoFalke · commit fa56c421be04 · 2023-09-15T14:34:24.000+02:00
This is a refactor.
diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp
@@ -79,7 +79,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe
         return false;
     }
 
-    CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true), CLIENT_VERSION};
+    CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)};
     if (file.IsNull()) {
         return error("%s: OpenBlockFile failed", __func__);
     }
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
@@ -459,7 +459,7 @@ bool BlockManager::LoadBlockIndexDB()
     }
     for (std::set<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {
         FlatFilePos pos(*it, 0);
-        if (AutoFile{OpenBlockFile(pos, true)}.IsNull()) {
+        if (OpenBlockFile(pos, true).IsNull()) {
             return false;
         }
     }
@@ -592,7 +592,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)
 bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const
 {
     // Open history file to append
-    AutoFile fileout{OpenUndoFile(pos)};
+    CAutoFile fileout{OpenUndoFile(pos)};
     if (fileout.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
@@ -627,7 +627,7 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in
     }
 
     // Open history file to read
-    AutoFile filein{OpenUndoFile(pos, true)};
+    CAutoFile filein{OpenUndoFile(pos, true)};
     if (filein.IsNull()) {
         return error("%s: OpenUndoFile failed", __func__);
     }
@@ -715,15 +715,15 @@ FlatFileSeq BlockManager::UndoFileSeq() const
     return FlatFileSeq(m_opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE);
 }
 
-FILE* BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const
+CAutoFile BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const
 {
-    return BlockFileSeq().Open(pos, fReadOnly);
+    return CAutoFile{BlockFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};
 }
 
 /** Open an undo file (rev?????.dat) */
-FILE* BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const
+CAutoFile BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const
 {
-    return UndoFileSeq().Open(pos, fReadOnly);
+    return CAutoFile{UndoFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};
 }
 
 fs::path BlockManager::GetBlockPosFilename(const FlatFilePos& pos) const
@@ -824,7 +824,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP
 bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const
 {
     // Open history file to append
-    CAutoFile fileout{OpenBlockFile(pos), CLIENT_VERSION};
+    CAutoFile fileout{OpenBlockFile(pos)};
     if (fileout.IsNull()) {
         return error("WriteBlockToDisk: OpenBlockFile failed");
     }
@@ -880,7 +880,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons
     block.SetNull();
 
     // Open history file to read
-    CAutoFile filein{OpenBlockFile(pos, true), CLIENT_VERSION};
+    CAutoFile filein{OpenBlockFile(pos, true)};
     if (filein.IsNull()) {
         return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());
     }
@@ -923,7 +923,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF
 {
     FlatFilePos hpos = pos;
     hpos.nPos -= 8; // Seek back 8 bytes for meta header
-    AutoFile filein{OpenBlockFile(hpos, true)};
+    CAutoFile filein{OpenBlockFile(hpos, true)};
     if (filein.IsNull()) {
         return error("%s: OpenBlockFile failed for %s", __func__, pos.ToString());
     }
@@ -1015,7 +1015,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile
                 if (!fs::exists(chainman.m_blockman.GetBlockPosFilename(pos))) {
                     break; // No block files left to reindex
                 }
-                CAutoFile file{chainman.m_blockman.OpenBlockFile(pos, true), CLIENT_VERSION};
+                CAutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)};
                 if (file.IsNull()) {
                     break; // This error is logged in OpenBlockFile
                 }
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
@@ -29,6 +29,7 @@
 #include <vector>
 
 class BlockValidationState;
+class CAutoFile;
 class CBlock;
 class CBlockFileInfo;
 class CBlockUndo;
@@ -126,7 +127,7 @@ class BlockManager
     FlatFileSeq BlockFileSeq() const;
     FlatFileSeq UndoFileSeq() const;
 
-    FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const;
+    CAutoFile OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false) const;
 
     bool WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const;
     bool UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const;
@@ -278,7 +279,7 @@ class BlockManager
     void UpdatePruneLock(const std::string& name, const PruneLockInfo& lock_info) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
 
     /** Open a block file (blk?????.dat) */
-    FILE* OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;
+    CAutoFile OpenBlockFile(const FlatFilePos& pos, bool fReadOnly = false) const;
 
     /** Translation to a filesystem path */
     fs::path GetBlockPosFilename(const FlatFilePos& pos) const;
diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp
@@ -74,13 +74,13 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain
     // Check that the file is not unlinked after ScanAndUnlinkAlreadyPrunedFiles
     // if m_have_pruned is not yet set
     WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
-    BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(pos, true)).IsNull());
+    BOOST_CHECK(!blockman.OpenBlockFile(pos, true).IsNull());
 
     // Check that the file is unlinked after ScanAndUnlinkAlreadyPrunedFiles
     // once m_have_pruned is set
     blockman.m_have_pruned = true;
     WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
-    BOOST_CHECK(AutoFile(blockman.OpenBlockFile(pos, true)).IsNull());
+    BOOST_CHECK(blockman.OpenBlockFile(pos, true).IsNull());
 
     // Check that calling with already pruned files doesn't cause an error
     WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles());
@@ -90,7 +90,7 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain
     BOOST_CHECK_NE(old_tip, new_tip);
     const int new_file_number{WITH_LOCK(chainman->GetMutex(), return new_tip->GetBlockPos().nFile)};
     const FlatFilePos new_pos(new_file_number, 0);
-    BOOST_CHECK(!AutoFile(blockman.OpenBlockFile(new_pos, true)).IsNull());
+    BOOST_CHECK(!blockman.OpenBlockFile(new_pos, true).IsNull());
 }
 
 BOOST_FIXTURE_TEST_CASE(blockmanager_block_data_availability, TestChain100Setup)

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ bool TxIndex::FindTx(const uint256& tx_hash, uint256& block_hash, CTransactionRe`
`79`	`79`	`return false;`
`80`	`80`	`}`
`81`	`81`
`82`		`- CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true), CLIENT_VERSION};`
	`82`	`+ CAutoFile file{m_chainstate->m_blockman.OpenBlockFile(postx, true)};`
`83`	`83`	`if (file.IsNull()) {`
`84`	`84`	`return error("%s: OpenBlockFile failed", __func__);`
`85`	`85`	`}`
Original file line number	Diff line number	Diff line change
`@@ -459,7 +459,7 @@ bool BlockManager::LoadBlockIndexDB()`
`459`	`459`	`}`
`460`	`460`	`for (std::set<int>::iterator it = setBlkDataFiles.begin(); it != setBlkDataFiles.end(); it++) {`
`461`	`461`	`FlatFilePos pos(*it, 0);`
`462`		`- if (AutoFile{OpenBlockFile(pos, true)}.IsNull()) {`
	`462`	`+ if (OpenBlockFile(pos, true).IsNull()) {`
`463`	`463`	`return false;`
`464`	`464`	`}`
`465`	`465`	`}`
`@@ -592,7 +592,7 @@ CBlockFileInfo* BlockManager::GetBlockFileInfo(size_t n)`
`592`	`592`	`bool BlockManager::UndoWriteToDisk(const CBlockUndo& blockundo, FlatFilePos& pos, const uint256& hashBlock) const`
`593`	`593`	`{`
`594`	`594`	`// Open history file to append`
`595`		`- AutoFile fileout{OpenUndoFile(pos)};`
	`595`	`+ CAutoFile fileout{OpenUndoFile(pos)};`
`596`	`596`	`if (fileout.IsNull()) {`
`597`	`597`	`return error("%s: OpenUndoFile failed", __func__);`
`598`	`598`	`}`
`@@ -627,7 +627,7 @@ bool BlockManager::UndoReadFromDisk(CBlockUndo& blockundo, const CBlockIndex& in`
`627`	`627`	`}`
`628`	`628`
`629`	`629`	`// Open history file to read`
`630`		`- AutoFile filein{OpenUndoFile(pos, true)};`
	`630`	`+ CAutoFile filein{OpenUndoFile(pos, true)};`
`631`	`631`	`if (filein.IsNull()) {`
`632`	`632`	`return error("%s: OpenUndoFile failed", __func__);`
`633`	`633`	`}`
`@@ -715,15 +715,15 @@ FlatFileSeq BlockManager::UndoFileSeq() const`
`715`	`715`	`return FlatFileSeq(m_opts.blocks_dir, "rev", UNDOFILE_CHUNK_SIZE);`
`716`	`716`	`}`
`717`	`717`
`718`		`-FILE* BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const`
	`718`	`+CAutoFile BlockManager::OpenBlockFile(const FlatFilePos& pos, bool fReadOnly) const`
`719`	`719`	`{`
`720`		`- return BlockFileSeq().Open(pos, fReadOnly);`
	`720`	`+ return CAutoFile{BlockFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};`
`721`	`721`	`}`
`722`	`722`
`723`	`723`	`/** Open an undo file (rev?????.dat) */`
`724`		`-FILE* BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const`
	`724`	`+CAutoFile BlockManager::OpenUndoFile(const FlatFilePos& pos, bool fReadOnly) const`
`725`	`725`	`{`
`726`		`- return UndoFileSeq().Open(pos, fReadOnly);`
	`726`	`+ return CAutoFile{UndoFileSeq().Open(pos, fReadOnly), CLIENT_VERSION};`
`727`	`727`	`}`
`728`	`728`
`729`	`729`	`fs::path BlockManager::GetBlockPosFilename(const FlatFilePos& pos) const`
`@@ -824,7 +824,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP`
`824`	`824`	`bool BlockManager::WriteBlockToDisk(const CBlock& block, FlatFilePos& pos) const`
`825`	`825`	`{`
`826`	`826`	`// Open history file to append`
`827`		`- CAutoFile fileout{OpenBlockFile(pos), CLIENT_VERSION};`
	`827`	`+ CAutoFile fileout{OpenBlockFile(pos)};`
`828`	`828`	`if (fileout.IsNull()) {`
`829`	`829`	`return error("WriteBlockToDisk: OpenBlockFile failed");`
`830`	`830`	`}`
`@@ -880,7 +880,7 @@ bool BlockManager::ReadBlockFromDisk(CBlock& block, const FlatFilePos& pos) cons`
`880`	`880`	`block.SetNull();`
`881`	`881`
`882`	`882`	`// Open history file to read`
`883`		`- CAutoFile filein{OpenBlockFile(pos, true), CLIENT_VERSION};`
	`883`	`+ CAutoFile filein{OpenBlockFile(pos, true)};`
`884`	`884`	`if (filein.IsNull()) {`
`885`	`885`	`return error("ReadBlockFromDisk: OpenBlockFile failed for %s", pos.ToString());`
`886`	`886`	`}`
`@@ -923,7 +923,7 @@ bool BlockManager::ReadRawBlockFromDisk(std::vector<uint8_t>& block, const FlatF`
`923`	`923`	`{`
`924`	`924`	`FlatFilePos hpos = pos;`
`925`	`925`	`hpos.nPos -= 8; // Seek back 8 bytes for meta header`
`926`		`- AutoFile filein{OpenBlockFile(hpos, true)};`
	`926`	`+ CAutoFile filein{OpenBlockFile(hpos, true)};`
`927`	`927`	`if (filein.IsNull()) {`
`928`	`928`	`return error("%s: OpenBlockFile failed for %s", __func__, pos.ToString());`
`929`	`929`	`}`
`@@ -1015,7 +1015,7 @@ void ImportBlocks(ChainstateManager& chainman, std::vector<fs::path> vImportFile`
`1015`	`1015`	`if (!fs::exists(chainman.m_blockman.GetBlockPosFilename(pos))) {`
`1016`	`1016`	`break; // No block files left to reindex`
`1017`	`1017`	`}`
`1018`		`- CAutoFile file{chainman.m_blockman.OpenBlockFile(pos, true), CLIENT_VERSION};`
	`1018`	`+ CAutoFile file{chainman.m_blockman.OpenBlockFile(pos, true)};`
`1019`	`1019`	`if (file.IsNull()) {`
`1020`	`1020`	`break; // This error is logged in OpenBlockFile`
`1021`	`1021`	`}`