Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[1.0.1] Verify index >= 0 in apply_diff and fix warnings #722

Merged
merged 7 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 25 additions & 10 deletions libraries/libfc/include/fc/container/ordered_diff.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ namespace fc {
* @param Container container type for ordered diff and for diff_result
*/
template <typename T, typename SizeType = size_t, template<typename Y, typename...> typename Container = std::vector>
requires std::equality_comparable<T> && std::random_access_iterator<typename Container<T>::iterator>
requires std::equality_comparable<T>
&& std::random_access_iterator<typename Container<T>::iterator>
&& std::is_unsigned_v<SizeType>
&& std::is_unsigned_v<typename Container<T>::size_type>
&& (std::numeric_limits<typename Container<T>::size_type>::max() >= std::numeric_limits<SizeType>::max())
class ordered_diff {
public:
using size_type = SizeType;
using container_size_type = typename Container<T>::size_type;

struct diff_result {
Container<SizeType> remove_indexes;
Expand Down Expand Up @@ -99,23 +104,33 @@ class ordered_diff {
return result;
}

/// @param diff the diff_result created from diff(source, target), apply_diff(std::move(source), diff_result) => target
/// @param diff_in (input) the diff_result created from diff(source, target), apply_diff(std::move(source), diff_result) => target
/// @param container the source of diff(source, target) to modify using the diff_result to produce original target
/// @return the modified container now equal to original target
template <typename X>
requires std::same_as<std::decay_t<X>, diff_result>
static Container<T> apply_diff(Container<T>&& container, X&& diff) {
static Container<T> apply_diff(Container<T>&& container, X&& diff_in) {
X diff = std::forward<X>(diff_in);

// Remove from the source based on diff.remove_indexes
std::ptrdiff_t offset = 0;
for (SizeType index : diff.remove_indexes) {
FC_ASSERT(index + offset < container.size(), "diff.remove_indexes index ${idx} + offset ${o} not in range ${s}",
("idx", index)("o", offset)("s", container.size()));
container.erase(container.begin() + index + offset);
--offset;
for (container_size_type i = 0; i < diff.remove_indexes.size(); ++i) {
FC_ASSERT(i == 0 || diff.remove_indexes[i] > diff.remove_indexes[i-1],
"diff.remove_indexes not strictly monotonically increasing: current index ${c}, previous index ${p}",
("c", diff.remove_indexes[i])("p", diff.remove_indexes[i-1]));

assert(diff.remove_indexes[i] >= i);
auto updated_index = diff.remove_indexes[i] - i;
FC_ASSERT(updated_index < container.size(), "diff.remove_indexes index ${idx} - i ${i} not in range ${s}",
("idx", diff.remove_indexes[i])("i", i)("s", container.size()));
container.erase(container.begin() + updated_index);
}

// Insert into the source based on diff.insert_indexes
for (auto& [index, value] : diff.insert_indexes) {
for (container_size_type i = 0; i < diff.insert_indexes.size(); ++i) {
FC_ASSERT(i == 0 || diff.insert_indexes[i].first > diff.insert_indexes[i-1].first,
"diff.insert_indexes not strictly monotonically increasing: current index ${c}, previous index ${p}",
("c", diff.insert_indexes[i].first)("p", diff.insert_indexes[i-1].first));
auto& [index, value] = diff.insert_indexes[i];
FC_ASSERT(index <= container.size(), "diff.insert_indexes index ${idx} not in range ${s}",
("idx", index)("s", container.size()));
container.insert(container.begin() + index, std::move(value));
Expand Down
8 changes: 4 additions & 4 deletions libraries/libfc/test/test_ordered_diff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,15 @@ BOOST_AUTO_TEST_CASE(ordered_diff_test) try {
{ // All elements removed
vector<char> source = {'a', 'b', 'c', 'd', 'e'};
vector<char> target;
auto result = ordered_diff<char, int>::diff(source, target);
source = ordered_diff<char, int>::apply_diff(std::move(source), result);
auto result = ordered_diff<char, unsigned int>::diff(source, target);
source = ordered_diff<char, unsigned int>::apply_diff(std::move(source), result);
BOOST_TEST(source == target);
}
{ // All elements removed, size 1
vector<char> source = {'a'};
vector<char> target;
auto result = ordered_diff<char, int>::diff(source, target);
source = ordered_diff<char, int>::apply_diff(std::move(source), result);
auto result = ordered_diff<char, unsigned int>::diff(source, target);
source = ordered_diff<char, unsigned int>::apply_diff(std::move(source), result);
BOOST_TEST(source == target);
}
{ // All elements inserted
Expand Down