Since this struct is only used to pass data out of the module, what do you think about making all the members const, eg:
<details>
<summary>Diff</summary>
index bb3c22ebdb..dba0b3a4a6 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -940,19 +940,19 @@ std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& ad
if (!addr_info_temp) { return std::nullopt; }
const AddrInfo& addr_info = *addr_info_temp;
- AddressPosition entry;
if(addr_info.fInTried) {
- entry.tried = true;
- entry.multiplicity = 1;
- entry.bucket = addr_info.GetTriedBucket(nKey, m_asmap);
- entry.position = addr_info.GetBucketPosition(nKey, false, entry.bucket);
- return entry;
+ int bucket{addr_info.GetTriedBucket(nKey, m_asmap)};
+ return AddressPosition(/*tried=*/true,
+ /*multiplicity=*/1,
+ /*bucket=*/bucket,
+ /*position=*/addr_info.GetBucketPosition(nKey, false, bucket));
+ } else {
+ int bucket{addr_info.GetNewBucket(nKey, m_asmap)};
+ return AddressPosition(/*tried=*/false,
+ /*multiplicity=*/addr_info.nRefCount,
+ /*bucket=*/bucket,
+ /*position=*/addr_info.GetBucketPosition(nKey, true, bucket));
}
-
- entry.multiplicity = addr_info.nRefCount;
- entry.bucket = addr_info.GetNewBucket(nKey, m_asmap);
- entry.position = addr_info.GetBucketPosition(nKey, true, entry.bucket);
- return entry;
}
void AddrManImpl::Check() const
diff --git a/src/addrman.h b/src/addrman.h
index 7cda33c185..68399fbd0d 100644
--- a/src/addrman.h
+++ b/src/addrman.h
@@ -25,24 +25,28 @@ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0};
/** Test-only struct, capturing info about an address in AddrMan */
struct AddressPosition {
// Whether the address is in the new or tried table
- bool tried{false};
+ const bool tried;
// Addresses in the tried table should always have a multiplicity of 1.
// Addresses in the new table can have multiplicity between 1 and
// ADDRMAN_NEW_BUCKETS_PER_ADDRESS
- int multiplicity{0};
+ const int multiplicity;
// If the address is in the new table, the bucket and position are
// populated based on the first source who sent the address.
// In certain edge cases, this may not be where the address is currently
// located.
- int bucket{0};
- int position{0};
+ const int bucket;
+ const int position;
bool operator==(AddressPosition other) {
return std::tie(tried, multiplicity, bucket, position) ==
std::tie(other.tried, other.multiplicity, other.bucket, other.position);
}
+
+ explicit AddressPosition(bool tried_in, int multiplicity_in, int bucket_in, int position_in)
+ : tried{tried_in}, multiplicity{multiplicity_in}, bucket{bucket_in}, position{position_in} {}
+
};
/** Stochastic address manager
diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
index a4d57e3435..bf09d631d4 100644
--- a/src/test/addrman_tests.cpp
+++ b/src/test/addrman_tests.cpp
@@ -237,8 +237,8 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
addr.nTime = start_time + i;
addrman->Add({addr}, source);
}
- addr_pos = addrman->FindAddressEntry(addr).value();
- BOOST_CHECK_EQUAL(addr_pos.multiplicity, 8U);
+ auto addr_pos2 = addrman->FindAddressEntry(addr).value();
+ BOOST_CHECK_EQUAL(addr_pos2.multiplicity, 8U);
// multiplicity doesn't affect size
BOOST_CHECK_EQUAL(addrman->size(), 1U);
}
</details>
(When we move to c++20 we can remove the ctor definition and use designated initializers)
The advantage to this is that all the fields must be filled explicitly by FindAddressEntry() so there's no risk that we accidentally pass out a structure where one of the fields has been incorrectly set to the default value.
