From e1197fa963945ca53e235b55a17f8ffc500c1b92 Mon Sep 17 00:00:00 2001 From: Myles Watson Date: Fri, 29 Jan 2021 15:54:18 -0800 Subject: [PATCH] Packet: Remove non-standard iterator cases Clang-tidy was complaining about the non-const iterators. Bug: 180016989 Test: cert/run Tag: #gd-refactor Change-Id: I2223955ab35af4f20b9dccbc1313a4e66a4734fc --- gd/packet/iterator.cc | 18 ++----------- gd/packet/iterator.h | 11 ++++---- gd/packet/packet_view_unittest.cc | 54 ++++++++++++++++++++++++--------------- gd/packet/python3_module.cc | 4 +-- 4 files changed, 43 insertions(+), 44 deletions(-) diff --git a/gd/packet/iterator.cc b/gd/packet/iterator.cc index f924b055a..7551b794b 100644 --- a/gd/packet/iterator.cc +++ b/gd/packet/iterator.cc @@ -22,7 +22,7 @@ namespace bluetooth { namespace packet { template -Iterator::Iterator(std::forward_list data, size_t offset) { +Iterator::Iterator(const std::forward_list& data, size_t offset) { data_ = data; index_ = offset; begin_ = 0; @@ -46,13 +46,6 @@ Iterator& Iterator::operator+=(int offset) { } template -Iterator Iterator::operator++(int) { - auto itr(*this); - index_++; - return itr; -} - -template Iterator& Iterator::operator++() { index_++; return *this; @@ -78,14 +71,6 @@ Iterator& Iterator::operator-=(int offset) { } template -Iterator Iterator::operator--(int) { - auto itr(*this); - if (index_ != 0) index_--; - - return itr; -} - -template Iterator& Iterator::operator--() { if (index_ != 0) index_--; @@ -94,6 +79,7 @@ Iterator& Iterator::operator--() { template Iterator& Iterator::operator=(const Iterator& itr) { + if (this == &itr) return *this; this->data_ = itr.data_; this->begin_ = itr.begin_; this->end_ = itr.end_; diff --git a/gd/packet/iterator.h b/gd/packet/iterator.h index cba1dd7c1..13a277dfa 100644 --- a/gd/packet/iterator.h +++ b/gd/packet/iterator.h @@ -31,20 +31,18 @@ namespace packet { template class Iterator : public std::iterator { public: - Iterator(std::forward_list data, size_t offset); + Iterator(const std::forward_list& data, size_t offset); Iterator(const Iterator& itr) = default; virtual ~Iterator() = default; // All addition and subtraction operators are unbounded. Iterator operator+(int offset); Iterator& operator+=(int offset); - Iterator operator++(int); Iterator& operator++(); Iterator operator-(int offset); int operator-(Iterator& itr); Iterator& operator-=(int offset); - Iterator operator--(int); Iterator& operator--(); Iterator& operator=(const Iterator& itr); @@ -59,7 +57,6 @@ class Iterator : public std::iterator bool operator>=(const Iterator& itr) const; uint8_t operator*() const; - uint8_t operator->() const; size_t NumBytesRemaining() const; @@ -74,7 +71,8 @@ class Iterator : public std::iterator for (size_t i = 0; i < sizeof(FixedWidthPODType); i++) { size_t index = (little_endian ? i : sizeof(FixedWidthPODType) - i - 1); - value_ptr[index] = *((*this)++); + value_ptr[index] = this->operator*(); + this->operator++(); } return extracted_value; } @@ -84,7 +82,8 @@ class Iterator : public std::iterator T extracted_value{}; for (size_t i = 0; i < CustomFieldFixedSizeInterface::length(); i++) { size_t index = (little_endian ? i : CustomFieldFixedSizeInterface::length() - i - 1); - extracted_value.data()[index] = *((*this)++); + extracted_value.data()[index] = this->operator*(); + this->operator++(); } return extracted_value; } diff --git a/gd/packet/packet_view_unittest.cc b/gd/packet/packet_view_unittest.cc index 0ec2ef472..208784e79 100644 --- a/gd/packet/packet_view_unittest.cc +++ b/gd/packet/packet_view_unittest.cc @@ -55,13 +55,13 @@ template class IteratorTest : public ::testing::Test { public: IteratorTest() = default; - ~IteratorTest() = default; + ~IteratorTest() override = default; - void SetUp() { + void SetUp() override { packet = std::shared_ptr(new T({View(std::make_shared>(count_all), 0, count_all.size())})); } - void TearDown() { + void TearDown() override { packet.reset(); } @@ -74,7 +74,7 @@ TYPED_TEST_CASE(IteratorTest, PacketViewTypes); class IteratorExtractTest : public ::testing::Test { public: IteratorExtractTest() = default; - ~IteratorExtractTest() = default; + ~IteratorExtractTest() override = default; }; template @@ -90,7 +90,7 @@ TYPED_TEST_CASE(PacketViewTest, PacketViewTypes); class PacketViewMultiViewTest : public ::testing::Test { public: PacketViewMultiViewTest() = default; - ~PacketViewMultiViewTest() = default; + ~PacketViewMultiViewTest() override = default; const PacketView single_view = PacketView({View(std::make_shared>(count_all), 0, count_all.size())}); @@ -104,7 +104,7 @@ class PacketViewMultiViewTest : public ::testing::Test { class PacketViewMultiViewAppendTest : public ::testing::Test { public: PacketViewMultiViewAppendTest() = default; - ~PacketViewMultiViewAppendTest() = default; + ~PacketViewMultiViewAppendTest() override = default; class AppendedPacketView : public PacketView { public: @@ -126,7 +126,7 @@ class PacketViewMultiViewAppendTest : public ::testing::Test { class ViewTest : public ::testing::Test { public: ViewTest() = default; - ~ViewTest() = default; + ~ViewTest() override = default; }; TEST(IteratorExtractTest, extractLeTest) { @@ -192,8 +192,10 @@ TYPED_TEST(IteratorTest, preIncrementTest) { TYPED_TEST(IteratorTest, postIncrementTest) { auto plus_plus = this->packet->begin(); for (size_t i = 0; i < count_all.size(); i++) { - ASSERT_EQ(count_all[i], *(plus_plus++)) << "Post-increment test: Dereferenced iterator does not equal expected " - << "at index " << i; + ASSERT_EQ(count_all[i], plus_plus.operator*()) + << "Post-increment test: Dereferenced iterator does not equal expected " + << "at index " << i; + plus_plus.operator++(); } } @@ -228,10 +230,12 @@ TYPED_TEST(IteratorTest, preDecrementTest) { TYPED_TEST(IteratorTest, postDecrementTest) { auto minus_minus = this->packet->end(); - minus_minus--; + minus_minus.operator--(); for (size_t i = count_all.size() - 1; i > 0; i--) { - ASSERT_EQ(count_all[i], *(minus_minus--)) << "Post-decrement test: Dereferenced iterator does not equal expected " - << "at index " << i; + ASSERT_EQ(count_all[i], minus_minus.operator*()) + << "Post-decrement test: Dereferenced iterator does not equal expected " + << "at index " << i; + minus_minus.operator--(); } } @@ -308,14 +312,16 @@ TYPED_TEST(IteratorTest, numBytesRemainingTest) { size_t remaining = all.NumBytesRemaining(); for (size_t n = remaining; n > 0; n--) { ASSERT_EQ(remaining, all.NumBytesRemaining()); - all++; + all.operator++(); remaining--; } ASSERT_EQ(static_cast(0), all.NumBytesRemaining()); - ASSERT_DEATH(*(all++), ""); - all++; + all.operator++(); + ASSERT_DEATH(all.operator*(), ""); + all.operator++(); ASSERT_EQ(static_cast(0), all.NumBytesRemaining()); - ASSERT_DEATH(*(all++), ""); + all.operator++(); + ASSERT_DEATH(all.operator*(), ""); } TYPED_TEST(IteratorTest, subrangeTest) { @@ -521,7 +527,9 @@ TEST_F(PacketViewMultiViewTest, dereferenceTestLittleEndian) { auto single_itr = single_view.begin(); auto multi_itr = multi_view.begin(); for (size_t i = 0; i < single_view.size(); i++) { - ASSERT_EQ(*(single_itr++), *(multi_itr++)); + ASSERT_EQ(single_itr.operator*(), multi_itr.operator*()); + single_itr.operator++(); + multi_itr.operator++(); } ASSERT_DEATH(*multi_itr, ""); } @@ -530,7 +538,9 @@ TEST_F(PacketViewMultiViewTest, dereferenceTestBigEndian) { auto single_itr = single_view.begin(); auto multi_itr = multi_view.begin(); for (size_t i = 0; i < single_view.size(); i++) { - ASSERT_EQ(*(single_itr++), *(multi_itr++)); + ASSERT_EQ(single_itr.operator*(), multi_itr.operator*()); + single_itr.operator++(); + multi_itr.operator++(); } ASSERT_DEATH(*multi_itr, ""); } @@ -550,7 +560,9 @@ TEST_F(PacketViewMultiViewAppendTest, dereferenceTestLittleEndianAppend) { auto single_itr = single_view.begin(); auto multi_itr = multi_view.begin(); for (size_t i = 0; i < single_view.size(); i++) { - ASSERT_EQ(*(single_itr++), *(multi_itr++)); + ASSERT_EQ(single_itr.operator*(), multi_itr.operator*()); + single_itr.operator++(); + multi_itr.operator++(); } ASSERT_DEATH(*multi_itr, ""); } @@ -559,7 +571,9 @@ TEST_F(PacketViewMultiViewAppendTest, dereferenceTestBigEndianAppend) { auto single_itr = single_view.begin(); auto multi_itr = multi_view.begin(); for (size_t i = 0; i < single_view.size(); i++) { - ASSERT_EQ(*(single_itr++), *(multi_itr++)); + ASSERT_EQ(single_itr.operator*(), multi_itr.operator*()); + single_itr.operator++(); + multi_itr.operator++(); } ASSERT_DEATH(*multi_itr, ""); } diff --git a/gd/packet/python3_module.cc b/gd/packet/python3_module.cc index 11c24c01c..a5e770f1e 100644 --- a/gd/packet/python3_module.cc +++ b/gd/packet/python3_module.cc @@ -94,8 +94,8 @@ PYBIND11_MODULE(bluetooth_packets_python3, m) { })) .def("GetBytes", [](const PacketView view) { std::string result; - for (auto it = view.begin(); it != view.end(); it++) { - result += *it; + for (auto byte : view) { + result += byte; } return py::bytes(result); }); -- 2.11.0