[bt][common] clang-tidy ByteBuffer
Run clang-tidy -fix on ByteBuffer source. Fix cases of double-
construction of StaticByteBuffer. Fix reliance on .view() returning a
const-qualified BufferView.
Bug: 62803
Test: bt-host-unittests
Change-Id: I01c497bb77e767bf6e061344b09f3b44a80ecf28
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/443446
Reviewed-by: Marie Janssen 💖 <jamuraa@google.com>
Testability-Review: Marie Janssen 💖 <jamuraa@google.com>
Commit-Queue: Xo Wang <xow@google.com>
diff --git a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.cc b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.cc
index 474286a..e617557 100644
--- a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.cc
+++ b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.cc
@@ -22,7 +22,7 @@
return write_size;
}
-const BufferView ByteBuffer::view(size_t pos, size_t size) const {
+BufferView ByteBuffer::view(size_t pos, size_t size) const {
ZX_ASSERT_MSG(pos <= this->size(), "invalid offset (pos = %zu)", pos);
return BufferView(data() + pos, std::min(size, this->size() - pos));
}
@@ -65,7 +65,7 @@
return MutableBufferView(mutable_data() + pos, std::min(size, this->size() - pos));
}
-DynamicByteBuffer::DynamicByteBuffer() : buffer_size_(0u) {}
+DynamicByteBuffer::DynamicByteBuffer() = default;
DynamicByteBuffer::DynamicByteBuffer(size_t buffer_size) : buffer_size_(buffer_size) {
if (buffer_size == 0)
@@ -134,7 +134,7 @@
ZX_ASSERT_MSG(!size_ || bytes_, "|bytes_| cannot be nullptr if |size_| > 0");
}
-BufferView::BufferView() : size_(0u), bytes_(nullptr) {}
+BufferView::BufferView() = default;
const uint8_t* BufferView::data() const { return bytes_; }
@@ -156,7 +156,7 @@
ZX_ASSERT_MSG(!size_ || bytes_, "|bytes_| cannot be nullptr if |size_| > 0");
}
-MutableBufferView::MutableBufferView() : size_(0u), bytes_(nullptr) {}
+MutableBufferView::MutableBufferView() = default;
const uint8_t* MutableBufferView::data() const { return bytes_; }
diff --git a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.h b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.h
index 3de7f73..f85896f 100644
--- a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.h
+++ b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer.h
@@ -10,6 +10,7 @@
#include <array>
#include <cstdint>
+#include <limits>
#include <memory>
#include <string>
#include <string_view>
@@ -62,8 +63,7 @@
// A BufferView is only valid as long as the buffer that it points to is
// valid. Care should be taken to ensure that a BufferView does not outlive
// its backing buffer.
- const BufferView view(size_t pos = 0,
- size_t size = std::numeric_limits<std::size_t>::max()) const;
+ BufferView view(size_t pos = 0, size_t size = std::numeric_limits<std::size_t>::max()) const;
// Copies |size| bytes of this buffer into |out_buffer| starting at offset
// |pos| and returns the number of bytes that were copied. |out_buffer| must
@@ -208,7 +208,7 @@
// The class's |BufferSize| template parameter, if explicitly provided, will be checked against
// the number of initialization elements provided.
template <typename... T>
- StaticByteBuffer(T... bytes) : buffer_{{static_cast<uint8_t>(bytes)...}} {
+ explicit StaticByteBuffer(T... bytes) : buffer_{{static_cast<uint8_t>(bytes)...}} {
static_assert(BufferSize, "|BufferSize| must be non-zero");
static_assert(BufferSize == sizeof...(T), "|BufferSize| must match initializer list count");
}
@@ -261,7 +261,7 @@
// Copies the contents of |buffer|.
explicit DynamicByteBuffer(const ByteBuffer& buffer);
- explicit DynamicByteBuffer(const DynamicByteBuffer& buffer);
+ DynamicByteBuffer(const DynamicByteBuffer& buffer);
// Takes ownership of |buffer| and avoids allocating a new buffer. Since this
// constructor performs a simple assignment, the caller must make sure that
@@ -287,7 +287,7 @@
private:
// Pointer to the underlying buffer, which is owned and managed by us.
- size_t buffer_size_;
+ size_t buffer_size_ = 0u;
std::unique_ptr<uint8_t[]> buffer_;
};
@@ -319,8 +319,8 @@
const_iterator cend() const override;
private:
- size_t size_;
- const uint8_t* bytes_;
+ size_t size_ = 0u;
+ const uint8_t* bytes_ = nullptr;
};
// A ByteBuffer that does not own the memory that it points to but rather
@@ -351,8 +351,8 @@
void Fill(uint8_t value) override;
private:
- size_t size_;
- uint8_t* bytes_;
+ size_t size_ = 0u;
+ uint8_t* bytes_ = nullptr;
};
} // namespace bt
diff --git a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer_unittest.cc b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer_unittest.cc
index 14ee846..e7d2a9c 100644
--- a/src/connectivity/bluetooth/core/bt-host/common/byte_buffer_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/common/byte_buffer_unittest.cc
@@ -23,10 +23,11 @@
EXPECT_TRUE(ContainersEqual(kExpected, buffer));
// Moving will result in a copy.
- StaticByteBuffer<kBufferSize> buffer_copy = std::move(buffer);
- EXPECT_EQ(kBufferSize, buffer.size());
+ StaticByteBuffer<kBufferSize> buffer_copy =
+ std::move(buffer); // NOLINT(performance-move-const-arg)
+ EXPECT_EQ(kBufferSize, buffer.size()); // NOLINT(bugprone-use-after-move)
EXPECT_EQ(kBufferSize, buffer_copy.size());
- EXPECT_TRUE(ContainersEqual(kExpected, buffer));
+ EXPECT_TRUE(ContainersEqual(kExpected, buffer)); // NOLINT(bugprone-use-after-move)
EXPECT_TRUE(ContainersEqual(kExpected, buffer_copy));
// Copying should also copy.
@@ -67,9 +68,9 @@
// Moving will invalidate the source buffer.
DynamicByteBuffer buffer_moved = std::move(buffer);
- EXPECT_EQ(0u, buffer.size());
+ EXPECT_EQ(0u, buffer.size()); // NOLINT(bugprone-use-after-move)
EXPECT_EQ(kBufferSize, buffer_moved.size());
- EXPECT_EQ(nullptr, buffer.data());
+ EXPECT_EQ(nullptr, buffer.data()); // NOLINT(bugprone-use-after-move)
EXPECT_TRUE(ContainersEqual(kExpected, buffer_moved));
}
@@ -87,7 +88,7 @@
TEST(ByteBufferTest, DynamicByteBufferConstructFromBuffer) {
constexpr size_t kBufferSize = 3;
- StaticByteBuffer<kBufferSize> buffer({1, 2, 3});
+ StaticByteBuffer<kBufferSize> buffer(1, 2, 3);
DynamicByteBuffer dyn_buffer(buffer);
EXPECT_EQ(kBufferSize, dyn_buffer.size());
diff --git a/src/connectivity/bluetooth/core/bt-host/l2cap/bredr_signaling_channel.cc b/src/connectivity/bluetooth/core/bt-host/l2cap/bredr_signaling_channel.cc
index c95ccdf..5832e00 100644
--- a/src/connectivity/bluetooth/core/bt-host/l2cap/bredr_signaling_channel.cc
+++ b/src/connectivity/bluetooth/core/bt-host/l2cap/bredr_signaling_channel.cc
@@ -43,7 +43,7 @@
size_t sdu_offset = 0;
while (sdu_offset + sizeof(CommandHeader) <= sdu->size()) {
- auto& header_data = sdu->view(sdu_offset, sizeof(CommandHeader));
+ const auto header_data = sdu->view(sdu_offset, sizeof(CommandHeader));
SignalingPacket packet(&header_data);
uint16_t expected_payload_length = le16toh(packet.header().length);
@@ -55,7 +55,7 @@
return;
}
- auto& packet_data = sdu->view(sdu_offset, sizeof(CommandHeader) + expected_payload_length);
+ const auto packet_data = sdu->view(sdu_offset, sizeof(CommandHeader) + expected_payload_length);
cb(SignalingPacket(&packet_data, expected_payload_length));
sdu_offset += packet_data.size();
diff --git a/src/connectivity/bluetooth/core/bt-host/sm/security_request_phase_unittest.cc b/src/connectivity/bluetooth/core/bt-host/sm/security_request_phase_unittest.cc
index 0d70e79..e920446 100644
--- a/src/connectivity/bluetooth/core/bt-host/sm/security_request_phase_unittest.cc
+++ b/src/connectivity/bluetooth/core/bt-host/sm/security_request_phase_unittest.cc
@@ -75,7 +75,7 @@
TEST_F(SMP_SecurityRequestPhaseTest, MakeEncryptedBondableSecurityRequest) {
NewSecurityRequestPhase(SecurityRequestOptions{.requested_level = SecurityLevel::kEncrypted,
.bondable = BondableMode::Bondable});
- StaticByteBuffer kExpectedReq = {kSecurityRequest, AuthReq::kBondingFlag};
+ StaticByteBuffer kExpectedReq(kSecurityRequest, AuthReq::kBondingFlag);
async::PostTask(dispatcher(), [this] { security_request_phase()->Start(); });
ASSERT_TRUE(Expect(kExpectedReq));
EXPECT_EQ(SecurityLevel::kEncrypted, security_request_phase()->pending_security_request());
@@ -84,7 +84,7 @@
TEST_F(SMP_SecurityRequestPhaseTest, MakeAuthenticatedNonBondableSecurityRequest) {
NewSecurityRequestPhase(SecurityRequestOptions{.requested_level = SecurityLevel::kAuthenticated,
.bondable = BondableMode::NonBondable});
- StaticByteBuffer kExpectedReq = {kSecurityRequest, AuthReq::kMITM};
+ StaticByteBuffer kExpectedReq(kSecurityRequest, AuthReq::kMITM);
async::PostTask(dispatcher(), [this] { security_request_phase()->Start(); });
ASSERT_TRUE(Expect(kExpectedReq));
EXPECT_EQ(SecurityLevel::kAuthenticated, security_request_phase()->pending_security_request());
@@ -93,8 +93,8 @@
TEST_F(SMP_SecurityRequestPhaseTest, MakeSecureAuthenticatedBondableSecurityRequest) {
NewSecurityRequestPhase(
SecurityRequestOptions{.requested_level = SecurityLevel::kSecureAuthenticated});
- StaticByteBuffer kExpectedReq = {kSecurityRequest,
- AuthReq::kBondingFlag | AuthReq::kMITM | AuthReq::kSC};
+ StaticByteBuffer kExpectedReq(kSecurityRequest,
+ AuthReq::kBondingFlag | AuthReq::kMITM | AuthReq::kSC);
async::PostTask(dispatcher(), [this] {
security_request_phase()->Start();
;
