Skip to content

Commit 50b63a5

Browse files
committed
refactor: inline constant return value of CDBWrapper::WriteBatch
`WriteBatch` can only ever return `true` - its errors are handled by throwing a `throw dbwrapper_error` instead. The boolean return value is quite confusing, especially since it's symmetric with `CDBWrapper::Read`, which catches the exceptions and returns a boolean instead. We're removing the constant return value and inlining `true` for its usages.
1 parent 9b1a7c3 commit 50b63a5

File tree

9 files changed

+16
-13
lines changed

9 files changed

+16
-13
lines changed

src/dbwrapper.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ CDBWrapper::~CDBWrapper()
274274
DBContext().options.env = nullptr;
275275
}
276276

277-
bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
277+
void CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
278278
{
279279
const bool log_memory = LogAcceptCategory(BCLog::LEVELDB, BCLog::Level::Debug);
280280
double mem_before = 0;
@@ -288,7 +288,6 @@ bool CDBWrapper::WriteBatch(CDBBatch& batch, bool fSync)
288288
LogDebug(BCLog::LEVELDB, "WriteBatch memory usage: db=%s, before=%.1fMiB, after=%.1fMiB\n",
289289
m_name, mem_before, mem_after);
290290
}
291-
return true;
292291
}
293292

294293
size_t CDBWrapper::DynamicMemoryUsage() const

src/dbwrapper.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,8 @@ class CDBWrapper
234234
{
235235
CDBBatch batch(*this);
236236
batch.Write(key, value);
237-
return WriteBatch(batch, fSync);
237+
WriteBatch(batch, fSync);
238+
return true;
238239
}
239240

240241
//! @returns filesystem path to the on-disk data.
@@ -259,10 +260,11 @@ class CDBWrapper
259260
{
260261
CDBBatch batch(*this);
261262
batch.Erase(key);
262-
return WriteBatch(batch, fSync);
263+
WriteBatch(batch, fSync);
264+
return true;
263265
}
264266

265-
bool WriteBatch(CDBBatch& batch, bool fSync = false);
267+
void WriteBatch(CDBBatch& batch, bool fSync = false);
266268

267269
// Get an estimate of LevelDB memory usage (in bytes).
268270
size_t DynamicMemoryUsage() const;

src/index/base.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ bool BaseIndex::Commit()
262262
ok = CustomCommit(batch);
263263
if (ok) {
264264
GetDB().WriteBestBlock(batch, GetLocator(*m_chain, m_best_block_index.load()->GetBlockHash()));
265-
ok = GetDB().WriteBatch(batch);
265+
GetDB().WriteBatch(batch);
266266
}
267267
}
268268
if (!ok) {

src/index/blockfilterindex.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,7 @@ bool BlockFilterIndex::CustomRemove(const interfaces::BlockInfo& block)
338338
// But since this creates new references to the filter, the position should get updated here
339339
// atomically as well in case Commit fails.
340340
batch.Write(DB_FILTER_POS, m_next_filter_pos);
341-
if (!m_db->WriteBatch(batch)) return false;
341+
m_db->WriteBatch(batch);
342342

343343
// Update cached header to the previous block hash
344344
m_last_header = *Assert(ReadFilterHeader(block.height - 1, *Assert(block.prev_hash)));

src/index/coinstatsindex.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ bool CoinStatsIndex::CustomRemove(const interfaces::BlockInfo& block)
263263
return false;
264264
}
265265

266-
if (!m_db->WriteBatch(batch)) return false;
266+
m_db->WriteBatch(batch);
267267

268268
if (!ReverseBlock(block)) {
269269
return false; // failure cause logged internally

src/index/txindex.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ bool TxIndex::DB::WriteTxs(const std::vector<std::pair<Txid, CDiskTxPos>>& v_pos
4646
for (const auto& [txid, pos] : v_pos) {
4747
batch.Write(std::make_pair(DB_TXINDEX, txid.ToUint256()), pos);
4848
}
49-
return WriteBatch(batch);
49+
WriteBatch(batch);
50+
return true;
5051
}
5152

5253
TxIndex::TxIndex(std::unique_ptr<interfaces::Chain> chain, size_t n_cache_size, bool f_memory, bool f_wipe)

src/node/blockstorage.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ bool BlockTreeDB::WriteBatchSync(const std::vector<std::pair<int, const CBlockFi
8888
for (const CBlockIndex* bi : blockinfo) {
8989
batch.Write(std::make_pair(DB_BLOCK_INDEX, bi->GetBlockHash()), CDiskBlockIndex{bi});
9090
}
91-
return WriteBatch(batch, true);
91+
WriteBatch(batch, true);
92+
return true;
9293
}
9394

9495
bool BlockTreeDB::WriteFlag(const std::string& name, bool fValue)

src/test/dbwrapper_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch)
173173
// Remove key3 before it's even been written
174174
batch.Erase(key3);
175175

176-
BOOST_CHECK(dbw.WriteBatch(batch));
176+
dbw.WriteBatch(batch);
177177

178178
BOOST_CHECK(dbw.Read(key, res));
179179
BOOST_CHECK_EQUAL(res.ToString(), in.ToString());

src/txdb.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,9 @@ bool CCoinsViewDB::BatchWrite(CoinsViewCacheCursor& cursor, const uint256 &hashB
149149
batch.Write(DB_BEST_BLOCK, hashBlock);
150150

151151
LogDebug(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.ApproximateSize() * (1.0 / 1048576.0));
152-
bool ret = m_db->WriteBatch(batch);
152+
m_db->WriteBatch(batch);
153153
LogDebug(BCLog::COINDB, "Committed %u changed transaction outputs (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count);
154-
return ret;
154+
return true;
155155
}
156156

157157
size_t CCoinsViewDB::EstimateSize() const

0 commit comments

Comments
 (0)