Page MenuHomePhabricator
Differential D11494 Diff 33592 src/avalanche/voterecord.cpp

Changeset View

Standalone View

View Options

src/avalanche/voterecord.cpp

Show First 20 Lines • Show All 94 Lines • ▼ Show 20 Lines while (count < AVALANCHE_MAX_INFLIGHT_POLL) {
if (inflight.compare_exchange_weak(count, count + 1)) { if (inflight.compare_exchange_weak(count, count + 1)) {
return true; return true;
} }
} }
return false; return false;
} }
void VoteRecord::clearInflightRequest(uint8_t count) {
uint8_t currentInflight = inflight.load();
while (currentInflight > 0 &&
!inflight.compare_exchange_weak(
currentInflight,
count >= currentInflight ? 0 : currentInflight - count)) {
}
FabienUnsubmitted
Not Done
Inline Actions

You don't need the brackets

Fabien: You don't need the brackets
FabienUnsubmitted
Not Done
Inline Actions
uint8_t currentInflight = inflight.load();
- while (currentInflight > 0 &&
+ while (count <= currentInflight &&
!inflight.compare_exchange_weak(
- currentInflight,
- count >= currentInflight ? 0 : currentInflight - count)) {
- }
- }
+ currentInflight, currentInflight - count));}
} // namespace avalanche
Fabien:
sdulfariAuthorUnsubmitted
Done
Inline Actions

While the brackets are unnecessary, the linter puts a semicolon on its own line in this case. It looks weird so I picked this instead.

Your suggested implementation is incorrect. In the case that count > inflight, you want inflight to be 0.

sdulfari: While the brackets are unnecessary, the linter puts a semicolon on its own line in this case.
}
FabienUnsubmitted
Not Done
Inline Actions

Since now this can fail, it can make sense to return a bool like it is done in registerPoll

Fabien: Since now this can fail, it can make sense to return a bool like it is done in registerPoll
sdulfariAuthorUnsubmitted
Done
Inline Actions

The comparison failure can be spurious, but in that case we expect to loop until it's not. Note that registerPoll doesn't return false when compare_exchange_weak fails, but only when the condition on count isn't true. clearInflightRequest doesn't need to return false in a similar way, so having a return value doesn't make sense.

sdulfari: The comparison failure can be spurious, but in that case we expect to loop until it's not. Note…
} // namespace avalanche } // namespace avalanche