Since this struct is only used to pass data out of the module, what do you think about making all the members const, eg:
0index bb3c22ebdb..dba0b3a4a6 100644
1--- a/src/addrman.cpp
2+++ b/src/addrman.cpp
3@@ -940,19 +940,19 @@ std::optional<AddressPosition> AddrManImpl::FindAddressEntry_(const CAddress& ad
4 if (!addr_info_temp) { return std::nullopt; }
5 const AddrInfo& addr_info = *addr_info_temp;
6
7- AddressPosition entry;
8 if(addr_info.fInTried) {
9- entry.tried = true;
10- entry.multiplicity = 1;
11- entry.bucket = addr_info.GetTriedBucket(nKey, m_asmap);
12- entry.position = addr_info.GetBucketPosition(nKey, false, entry.bucket);
13- return entry;
14+ int bucket{addr_info.GetTriedBucket(nKey, m_asmap)};
15+ return AddressPosition(/*tried=*/true,
16+ /*multiplicity=*/1,
17+ /*bucket=*/bucket,
18+ /*position=*/addr_info.GetBucketPosition(nKey, false, bucket));
19+ } else {
20+ int bucket{addr_info.GetNewBucket(nKey, m_asmap)};
21+ return AddressPosition(/*tried=*/false,
22+ /*multiplicity=*/addr_info.nRefCount,
23+ /*bucket=*/bucket,
24+ /*position=*/addr_info.GetBucketPosition(nKey, true, bucket));
25 }
26-
27- entry.multiplicity = addr_info.nRefCount;
28- entry.bucket = addr_info.GetNewBucket(nKey, m_asmap);
29- entry.position = addr_info.GetBucketPosition(nKey, true, entry.bucket);
30- return entry;
31 }
32
33 void AddrManImpl::Check() const
34diff --git a/src/addrman.h b/src/addrman.h
35index 7cda33c185..68399fbd0d 100644
36--- a/src/addrman.h
37+++ b/src/addrman.h
38@@ -25,24 +25,28 @@ static constexpr int32_t DEFAULT_ADDRMAN_CONSISTENCY_CHECKS{0};
39 /** Test-only struct, capturing info about an address in AddrMan */
40 struct AddressPosition {
41 // Whether the address is in the new or tried table
42- bool tried{false};
43+ const bool tried;
44
45 // Addresses in the tried table should always have a multiplicity of 1.
46 // Addresses in the new table can have multiplicity between 1 and
47 // ADDRMAN_NEW_BUCKETS_PER_ADDRESS
48- int multiplicity{0};
49+ const int multiplicity;
50
51 // If the address is in the new table, the bucket and position are
52 // populated based on the first source who sent the address.
53 // In certain edge cases, this may not be where the address is currently
54 // located.
55- int bucket{0};
56- int position{0};
57+ const int bucket;
58+ const int position;
59
60 bool operator==(AddressPosition other) {
61 return std::tie(tried, multiplicity, bucket, position) ==
62 std::tie(other.tried, other.multiplicity, other.bucket, other.position);
63 }
64+
65+ explicit AddressPosition(bool tried_in, int multiplicity_in, int bucket_in, int position_in)
66+ : tried{tried_in}, multiplicity{multiplicity_in}, bucket{bucket_in}, position{position_in} {}
67+
68 };
69
70 /** Stochastic address manager
71diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp
72index a4d57e3435..bf09d631d4 100644
73--- a/src/test/addrman_tests.cpp
74+++ b/src/test/addrman_tests.cpp
75@@ -237,8 +237,8 @@ BOOST_AUTO_TEST_CASE(addrman_new_multiplicity)
76 addr.nTime = start_time + i;
77 addrman->Add({addr}, source);
78 }
79- addr_pos = addrman->FindAddressEntry(addr).value();
80- BOOST_CHECK_EQUAL(addr_pos.multiplicity, 8U);
81+ auto addr_pos2 = addrman->FindAddressEntry(addr).value();
82+ BOOST_CHECK_EQUAL(addr_pos2.multiplicity, 8U);
83 // multiplicity doesn't affect size
84 BOOST_CHECK_EQUAL(addrman->size(), 1U);
85 }
(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.
