Skip to content

Commit 15aaa81

Browse files
l0rincl0rinc
committed
coins, refactor: Remove direct GetFlags access
We don't need so much access to the internals of CCoinsCacheEntry, since many tests are just exercising invalid combinations this way. This implies that `AddFlags` has private access now.
1 parent 6b73369 commit 15aaa81

File tree

3 files changed

+42
-42
lines changed

3 files changed

+42
-42
lines changed

‎src/coins.h‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,21 @@ struct CCoinsCacheEntry
128128
CoinsCachePair* m_next{nullptr};
129129
uint8_t m_flags{0};
130130

131+
//! Adding a flag requires a reference to the sentinel of the flagged pair linked list.
132+
static void AddFlags(uint8_t flags, CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept
133+
{
134+
Assume(flags & (DIRTY | FRESH));
135+
if (!pair.second.m_flags) {
136+
Assume(!pair.second.m_prev && !pair.second.m_next);
137+
pair.second.m_prev = sentinel.second.m_prev;
138+
pair.second.m_next = &sentinel;
139+
sentinel.second.m_prev = &pair;
140+
pair.second.m_prev->second.m_next = &pair;
141+
}
142+
Assume(pair.second.m_prev && pair.second.m_next);
143+
pair.second.m_flags |= flags;
144+
}
145+
131146
public:
132147
Coin coin; // The actual cached data.
133148

@@ -159,22 +174,6 @@ struct CCoinsCacheEntry
159174
SetClean();
160175
}
161176

162-
//! Adding a flag also requires a self reference to the pair that contains
163-
//! this entry in the CCoinsCache map and a reference to the sentinel of the
164-
//! flagged pair linked list.
165-
static void AddFlags(uint8_t flags, CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept
166-
{
167-
Assume(flags & (DIRTY | FRESH));
168-
if (!pair.second.m_flags) {
169-
Assume(!pair.second.m_prev && !pair.second.m_next);
170-
pair.second.m_prev = sentinel.second.m_prev;
171-
pair.second.m_next = &sentinel;
172-
sentinel.second.m_prev = &pair;
173-
pair.second.m_prev->second.m_next = &pair;
174-
}
175-
Assume(pair.second.m_prev && pair.second.m_next);
176-
pair.second.m_flags |= flags;
177-
}
178177
static void SetDirty(CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept { AddFlags(DIRTY, pair, sentinel); }
179178
static void SetFresh(CoinsCachePair& pair, CoinsCachePair& sentinel) noexcept { AddFlags(FRESH, pair, sentinel); }
180179

@@ -186,7 +185,6 @@ struct CCoinsCacheEntry
186185
m_flags = 0;
187186
m_prev = m_next = nullptr;
188187
}
189-
uint8_t GetFlags() const noexcept { return m_flags; }
190188
bool IsDirty() const noexcept { return m_flags & DIRTY; }
191189
bool IsFresh() const noexcept { return m_flags & FRESH; }
192190

‎src/test/coins_tests.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -613,7 +613,9 @@ void GetCoinsMapEntry(const CCoinsMap& map, CAmount& value, char& flags, const C
613613
} else {
614614
value = it->second.coin.out.nValue;
615615
}
616-
flags = it->second.GetFlags();
616+
flags = 0;
617+
if (it->second.IsDirty()) flags |= DIRTY;
618+
if (it->second.IsFresh()) flags |= FRESH;
617619
assert(flags != NO_ENTRY);
618620
}
619621
}

‎src/test/coinscachepair_tests.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ std::list<CoinsCachePair> CreatePairs(CoinsCachePair& sentinel)
2121
auto node{std::prev(nodes.end())};
2222
CCoinsCacheEntry::SetDirty(*node, sentinel);
2323

24-
BOOST_CHECK_EQUAL(node->second.GetFlags(), CCoinsCacheEntry::DIRTY);
24+
BOOST_CHECK(node->second.IsDirty() && !node->second.IsFresh());
2525
BOOST_CHECK_EQUAL(node->second.Next(), &sentinel);
2626
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &(*node));
2727

@@ -48,7 +48,7 @@ BOOST_AUTO_TEST_CASE(linked_list_iteration)
4848
BOOST_CHECK_EQUAL(node, &sentinel);
4949

5050
// Check iterating through pairs is identical to iterating through a list
51-
// Clear the flags during iteration
51+
// Clear the state during iteration
5252
node = sentinel.second.Next();
5353
for (const auto& expected : nodes) {
5454
BOOST_CHECK_EQUAL(&expected, node);
@@ -63,7 +63,7 @@ BOOST_AUTO_TEST_CASE(linked_list_iteration)
6363

6464
// Delete the nodes from the list to make sure there are no dangling pointers
6565
for (auto it{nodes.begin()}; it != nodes.end(); it = nodes.erase(it)) {
66-
BOOST_CHECK_EQUAL(it->second.GetFlags(), 0);
66+
BOOST_CHECK(!it->second.IsDirty() && !it->second.IsFresh());
6767
}
6868
}
6969

@@ -74,8 +74,8 @@ BOOST_AUTO_TEST_CASE(linked_list_iterate_erase)
7474
auto nodes{CreatePairs(sentinel)};
7575

7676
// Check iterating through pairs is identical to iterating through a list
77-
// Erase the nodes as we iterate through, but don't clear flags
78-
// The flags will be cleared by the CCoinsCacheEntry's destructor
77+
// Erase the nodes as we iterate through, but don't clear state
78+
// The state will be cleared by the CCoinsCacheEntry's destructor
7979
auto node{sentinel.second.Next()};
8080
for (auto expected{nodes.begin()}; expected != nodes.end(); expected = nodes.erase(expected)) {
8181
BOOST_CHECK_EQUAL(&(*expected), node);
@@ -104,19 +104,19 @@ BOOST_AUTO_TEST_CASE(linked_list_random_deletion)
104104
// sentinel->n1->n3->n4->sentinel
105105
nodes.erase(n2);
106106
// Check that n1 now points to n3, and n3 still points to n4
107-
// Also check that flags were not altered
108-
BOOST_CHECK_EQUAL(n1->second.GetFlags(), CCoinsCacheEntry::DIRTY);
107+
// Also check that state was not altered
108+
BOOST_CHECK(n1->second.IsDirty() && !n1->second.IsFresh());
109109
BOOST_CHECK_EQUAL(n1->second.Next(), &(*n3));
110-
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
110+
BOOST_CHECK(n3->second.IsDirty() && !n3->second.IsFresh());
111111
BOOST_CHECK_EQUAL(n3->second.Next(), &(*n4));
112112
BOOST_CHECK_EQUAL(n3->second.Prev(), &(*n1));
113113

114114
// Delete n1
115115
// sentinel->n3->n4->sentinel
116116
nodes.erase(n1);
117117
// Check that sentinel now points to n3, and n3 still points to n4
118-
// Also check that flags were not altered
119-
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
118+
// Also check that state was not altered
119+
BOOST_CHECK(n3->second.IsDirty() && !n3->second.IsFresh());
120120
BOOST_CHECK_EQUAL(sentinel.second.Next(), &(*n3));
121121
BOOST_CHECK_EQUAL(n3->second.Next(), &(*n4));
122122
BOOST_CHECK_EQUAL(n3->second.Prev(), &sentinel);
@@ -125,8 +125,8 @@ BOOST_AUTO_TEST_CASE(linked_list_random_deletion)
125125
// sentinel->n3->sentinel
126126
nodes.erase(n4);
127127
// Check that sentinel still points to n3, and n3 points to sentinel
128-
// Also check that flags were not altered
129-
BOOST_CHECK_EQUAL(n3->second.GetFlags(), CCoinsCacheEntry::DIRTY);
128+
// Also check that state was not altered
129+
BOOST_CHECK(n3->second.IsDirty() && !n3->second.IsFresh());
130130
BOOST_CHECK_EQUAL(sentinel.second.Next(), &(*n3));
131131
BOOST_CHECK_EQUAL(n3->second.Next(), &sentinel);
132132
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &(*n3));
@@ -139,56 +139,56 @@ BOOST_AUTO_TEST_CASE(linked_list_random_deletion)
139139
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &sentinel);
140140
}
141141

142-
BOOST_AUTO_TEST_CASE(linked_list_add_flags)
142+
BOOST_AUTO_TEST_CASE(linked_list_set_state)
143143
{
144144
CoinsCachePair sentinel;
145145
sentinel.second.SelfRef(sentinel);
146146
CoinsCachePair n1;
147147
CoinsCachePair n2;
148148

149-
// Check that adding DIRTY flag inserts it into linked list and sets flags
149+
// Check that setting DIRTY inserts it into linked list and sets state
150150
CCoinsCacheEntry::SetDirty(n1, sentinel);
151-
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
151+
BOOST_CHECK(n1.second.IsDirty() && !n1.second.IsFresh());
152152
BOOST_CHECK_EQUAL(n1.second.Next(), &sentinel);
153153
BOOST_CHECK_EQUAL(n1.second.Prev(), &sentinel);
154154
BOOST_CHECK_EQUAL(sentinel.second.Next(), &n1);
155155
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n1);
156156

157-
// Check that adding FRESH flag on new node inserts it after n1
157+
// Check that setting FRESH on new node inserts it after n1
158158
CCoinsCacheEntry::SetFresh(n2, sentinel);
159-
BOOST_CHECK_EQUAL(n2.second.GetFlags(), CCoinsCacheEntry::FRESH);
159+
BOOST_CHECK(n2.second.IsFresh() && !n2.second.IsDirty());
160160
BOOST_CHECK_EQUAL(n2.second.Next(), &sentinel);
161161
BOOST_CHECK_EQUAL(n2.second.Prev(), &n1);
162162
BOOST_CHECK_EQUAL(n1.second.Next(), &n2);
163163
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n2);
164164

165-
// Check that we can add extra flags, but they don't change our position
165+
// Check that we can set extra state, but they don't change our position
166166
CCoinsCacheEntry::SetFresh(n1, sentinel);
167-
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY | CCoinsCacheEntry::FRESH);
167+
BOOST_CHECK(n1.second.IsDirty() && n1.second.IsFresh());
168168
BOOST_CHECK_EQUAL(n1.second.Next(), &n2);
169169
BOOST_CHECK_EQUAL(n1.second.Prev(), &sentinel);
170170
BOOST_CHECK_EQUAL(sentinel.second.Next(), &n1);
171171
BOOST_CHECK_EQUAL(n2.second.Prev(), &n1);
172172

173-
// Check that we can clear flags then re-add them
173+
// Check that we can clear state then re-set it
174174
n1.second.SetClean();
175-
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
175+
BOOST_CHECK(!n1.second.IsDirty() && !n1.second.IsFresh());
176176
BOOST_CHECK_EQUAL(sentinel.second.Next(), &n2);
177177
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n2);
178178
BOOST_CHECK_EQUAL(n2.second.Next(), &sentinel);
179179
BOOST_CHECK_EQUAL(n2.second.Prev(), &sentinel);
180180

181-
// Check that calling `SetClean` with 0 flags has no effect
181+
// Calling `SetClean` a second time has no effect
182182
n1.second.SetClean();
183-
BOOST_CHECK_EQUAL(n1.second.GetFlags(), 0);
183+
BOOST_CHECK(!n1.second.IsDirty() && !n1.second.IsFresh());
184184
BOOST_CHECK_EQUAL(sentinel.second.Next(), &n2);
185185
BOOST_CHECK_EQUAL(sentinel.second.Prev(), &n2);
186186
BOOST_CHECK_EQUAL(n2.second.Next(), &sentinel);
187187
BOOST_CHECK_EQUAL(n2.second.Prev(), &sentinel);
188188

189189
// Adding DIRTY re-inserts it after n2
190190
CCoinsCacheEntry::SetDirty(n1, sentinel);
191-
BOOST_CHECK_EQUAL(n1.second.GetFlags(), CCoinsCacheEntry::DIRTY);
191+
BOOST_CHECK(n1.second.IsDirty() && !n1.second.IsFresh());
192192
BOOST_CHECK_EQUAL(n2.second.Next(), &n1);
193193
BOOST_CHECK_EQUAL(n1.second.Prev(), &n2);
194194
BOOST_CHECK_EQUAL(n1.second.Next(), &sentinel);

0 commit comments

Comments
 (0)