Skip to content

Commit

Permalink
Fix negative-wraparound issue when converting size_t to ptrdiff_t whi…
Browse files Browse the repository at this point in the history
…ch is used by std::views::take.

This issue was flagged by VisualStudio debug asserts
  • Loading branch information
clemahieu committed Sep 3, 2024
1 parent 8c8a69b commit d975781
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 7 deletions.
4 changes: 2 additions & 2 deletions nano/node/active_elections.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ void nano::active_elections::request_confirm (nano::unique_lock<nano::mutex> & l
lock_a.unlock ();

nano::confirmation_solicitor solicitor (node.network, node.config);
solicitor.prepare (node.rep_crawler.principal_representatives (std::numeric_limits<std::size_t>::max ()));
solicitor.prepare (node.rep_crawler.principal_representatives (std::numeric_limits<std::ptrdiff_t>::max ()));

std::size_t unconfirmed_count_l (0);
nano::timer<std::chrono::milliseconds> elapsed (nano::timer_state::started);
Expand Down Expand Up @@ -652,4 +652,4 @@ nano::stat::type nano::to_stat_type (nano::election_state state)
nano::stat::detail nano::to_stat_detail (nano::election_status_type type)
{
return nano::enum_util::cast<nano::stat::detail> (type);
}
}
8 changes: 5 additions & 3 deletions nano/node/repcrawler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,8 +435,9 @@ nano::uint128_t nano::rep_crawler::total_weight () const
return result;
}

std::vector<nano::representative> nano::rep_crawler::representatives (std::size_t count, nano::uint128_t const minimum_weight, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version)
std::vector<nano::representative> nano::rep_crawler::representatives (std::ptrdiff_t count, nano::uint128_t const minimum_weight, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version)
{
debug_assert (count >= 0);
auto const version_min = minimum_protocol_version.value_or (node.network_params.network.protocol_version_min);

nano::lock_guard<nano::mutex> lock{ mutex };
Expand All @@ -452,15 +453,16 @@ std::vector<nano::representative> nano::rep_crawler::representatives (std::size_
}

std::vector<nano::representative> result;
for (auto const & [weight, rep] : ordered | std::views::take (count))
for (auto const & [weight, rep] : ordered | std::views::take (/* Signed type*/ count))
{
result.push_back ({ rep.account, rep.channel });
}
return result;
}

std::vector<nano::representative> nano::rep_crawler::principal_representatives (std::size_t count, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version)
std::vector<nano::representative> nano::rep_crawler::principal_representatives (std::ptrdiff_t count, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version)
{
debug_assert (count >= 0);
return representatives (count, node.minimum_principal_weight (), minimum_protocol_version);
}

Expand Down
4 changes: 2 additions & 2 deletions nano/node/repcrawler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,11 @@ class rep_crawler final

/** Request a list of the top \p count known representatives in descending order of weight, with at least \p weight_a voting weight, and optionally with a minimum version \p minimum_protocol_version
*/
std::vector<representative> representatives (std::size_t count = std::numeric_limits<std::size_t>::max (), nano::uint128_t minimum_weight = 0, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version = {});
std::vector<representative> representatives (std::ptrdiff_t count = std::numeric_limits<std::ptrdiff_t>::max (), nano::uint128_t minimum_weight = 0, std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version = {});

/** Request a list of the top \p count known principal representatives in descending order of weight, optionally with a minimum version \p minimum_protocol_version
*/
std::vector<representative> principal_representatives (std::size_t count = std::numeric_limits<std::size_t>::max (), std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version = {});
std::vector<representative> principal_representatives (std::ptrdiff_t count = std::numeric_limits<std::ptrdiff_t>::max (), std::optional<decltype (nano::network_constants::protocol_version)> const & minimum_protocol_version = {});

/** Total number of representatives */
std::size_t representative_count ();
Expand Down

0 comments on commit d975781

Please sign in to comment.