Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 1a0edfe4 authored by Myles Watson's avatar Myles Watson Committed by Chris Manton
Browse files

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
parent 38220e2e

system/gd/packet/iterator.cc

+2 −16
Original line number Diff line number Diff line
@@ -22,7 +22,7 @@ namespace bluetooth { 
namespace packet {



template <bool little_endian>

Iterator<little_endian>::Iterator(std::forward_list<View> data, size_t offset) {

Iterator<little_endian>::Iterator(const std::forward_list<View>& data, size_t offset) {

  data_ = data;

  index_ = offset;

  begin_ = 0;
@@ -45,13 +45,6 @@ Iterator<little_endian>& Iterator<little_endian>::operator+=(int offset) { 
  return *this;

}



template <bool little_endian>

Iterator<little_endian> Iterator<little_endian>::operator++(int) {

  auto itr(*this);

  index_++;

  return itr;

}



template <bool little_endian>

Iterator<little_endian>& Iterator<little_endian>::operator++() {

  index_++;
@@ -77,14 +70,6 @@ Iterator<little_endian>& Iterator<little_endian>::operator-=(int offset) { 
  return *this;

}



template <bool little_endian>

Iterator<little_endian> Iterator<little_endian>::operator--(int) {

  auto itr(*this);

  if (index_ != 0) index_--;



  return itr;

}



template <bool little_endian>

Iterator<little_endian>& Iterator<little_endian>::operator--() {

  if (index_ != 0) index_--;
@@ -94,6 +79,7 @@ Iterator<little_endian>& Iterator<little_endian>::operator--() { 


template <bool little_endian>

Iterator<little_endian>& Iterator<little_endian>::operator=(const Iterator<little_endian>& itr) {

  if (this == &itr) return *this;

  this->data_ = itr.data_;

  this->begin_ = itr.begin_;

  this->end_ = itr.end_;

system/gd/packet/iterator.h

+5 −6
Original line number Diff line number Diff line
@@ -31,20 +31,18 @@ namespace packet { 
template <bool little_endian>

class Iterator : public std::iterator<std::random_access_iterator_tag, uint8_t> {

 public:

  Iterator(std::forward_list<View> data, size_t offset);

  Iterator(const std::forward_list<View>& 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<std::random_access_iterator_tag, uint8_t> 
  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<std::random_access_iterator_tag, uint8_t> 


    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<std::random_access_iterator_tag, uint8_t> 
    T extracted_value{};

    for (size_t i = 0; i < CustomFieldFixedSizeInterface<T>::length(); i++) {

      size_t index = (little_endian ? i : CustomFieldFixedSizeInterface<T>::length() - i - 1);

      extracted_value.data()[index] = *((*this)++);

      extracted_value.data()[index] = this->operator*();

      this->operator++();

    }

    return extracted_value;

  }

system/gd/packet/packet_view_unittest.cc

+34 −20
Original line number Diff line number Diff line
@@ -55,13 +55,13 @@ template <typename T> 
class IteratorTest : public ::testing::Test {

 public:

  IteratorTest() = default;

  ~IteratorTest() = default;

  ~IteratorTest() override = default;



  void SetUp() {

  void SetUp() override {

    packet = std::shared_ptr<T>(new T({View(std::make_shared<const vector<uint8_t>>(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 <typename T>
@@ -90,7 +90,7 @@ TYPED_TEST_CASE(PacketViewTest, PacketViewTypes); 
class PacketViewMultiViewTest : public ::testing::Test {

 public:

  PacketViewMultiViewTest() = default;

  ~PacketViewMultiViewTest() = default;

  ~PacketViewMultiViewTest() override = default;



  const PacketView<true> single_view =

      PacketView<true>({View(std::make_shared<const vector<uint8_t>>(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<true> {

   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 "

    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 "

    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<size_t>(0), all.NumBytesRemaining());

  ASSERT_DEATH(*(all++), "");

  all++;

  all.operator++();

  ASSERT_DEATH(all.operator*(), "");

  all.operator++();

  ASSERT_EQ(static_cast<size_t>(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, "");

}

system/gd/packet/python3_module.cc

+2 −2
Original line number Diff line number Diff line
@@ -94,8 +94,8 @@ PYBIND11_MODULE(bluetooth_packets_python3, m) { 
      }))

      .def("GetBytes", [](const PacketView<kLittleEndian> 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);

      });