rtc::Buffer: Remove backwards compatibility band-aids

This CL makes two changes to rtc::Buffer that have had to wait for
Chromium's use of it to be modernized:

  1. Change default return type of rtc::Buffer::data() from char* to
     uint8_t*. uint8_t is a more natural type for bytes, and won't
     accidentally convert to a string. (Chromium previously expected
     the default return type to be char, which is why
     rtc::Buffer::data() initially got char as default return type in
     9478437f, but that's been fixed now.)

  2. Stop accepting void* inputs in constructors and methods. While
     this is convenient, it's also dangerous since any pointer type
     will implicitly convert to void*.

(This was previously committed (9e1a6d7c) but had to be reverted
(cbf09274) because Chromium on Android wasn't quite ready for it).

TBR=tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/47109004

Cr-Commit-Position: refs/heads/master@{#9132}
This commit is contained in:
Karl Wiberg 2015-05-04 14:54:55 +02:00
parent f75f0cf36a
commit c56ac1ec29
4 changed files with 18 additions and 33 deletions

View File

@ -512,7 +512,7 @@ bool BaseChannel::SendPacket(bool rtcp, rtc::Buffer* packet,
// Protect if needed. // Protect if needed.
if (srtp_filter_.IsActive()) { if (srtp_filter_.IsActive()) {
bool res; bool res;
uint8_t* data = packet->data<uint8_t>(); uint8_t* data = packet->data();
int len = static_cast<int>(packet->size()); int len = static_cast<int>(packet->size());
if (!rtcp) { if (!rtcp) {
// If ENABLE_EXTERNAL_AUTH flag is on then packet authentication is not done // If ENABLE_EXTERNAL_AUTH flag is on then packet authentication is not done

View File

@ -40,20 +40,6 @@ struct ByteType {
using t = decltype(F(static_cast<T*>(nullptr))); using t = decltype(F(static_cast<T*>(nullptr)));
}; };
// Deprecated: Accept void* in addition to the byte-sized types.
// TODO(kwiberg): Remove once Chromium doesn't need this anymore.
template <typename T>
struct ByteTypeOrVoid {
private:
static int F(uint8_t*);
static int F(int8_t*);
static int F(char*);
static int F(void*);
public:
using t = decltype(F(static_cast<T*>(nullptr)));
};
} // namespace internal } // namespace internal
// Basic buffer class, can be grown and shrunk dynamically. // Basic buffer class, can be grown and shrunk dynamically.
@ -70,10 +56,10 @@ class Buffer final {
// Construct a buffer and copy the specified number of bytes into it. The // Construct a buffer and copy the specified number of bytes into it. The
// source array may be (const) uint8_t*, int8_t*, or char*. // source array may be (const) uint8_t*, int8_t*, or char*.
template <typename T, typename internal::ByteTypeOrVoid<T>::t = 0> template <typename T, typename internal::ByteType<T>::t = 0>
Buffer(const T* data, size_t size) Buffer(const T* data, size_t size)
: Buffer(data, size, size) {} : Buffer(data, size, size) {}
template <typename T, typename internal::ByteTypeOrVoid<T>::t = 0> template <typename T, typename internal::ByteType<T>::t = 0>
Buffer(const T* data, size_t size, size_t capacity) Buffer(const T* data, size_t size, size_t capacity)
: Buffer(size, capacity) { : Buffer(size, capacity) {
std::memcpy(data_.get(), data, size); std::memcpy(data_.get(), data, size);
@ -86,15 +72,14 @@ class Buffer final {
~Buffer(); ~Buffer();
// Get a pointer to the data. Just .data() will give you a (const) char*, // Get a pointer to the data. Just .data() will give you a (const) uint8_t*,
// but you may also use .data<int8_t>() and .data<uint8_t>(). // but you may also use .data<int8_t>() and .data<char>().
// TODO(kwiberg): Change default to uint8_t template <typename T = uint8_t, typename internal::ByteType<T>::t = 0>
template <typename T = char, typename internal::ByteType<T>::t = 0>
const T* data() const { const T* data() const {
assert(IsConsistent()); assert(IsConsistent());
return reinterpret_cast<T*>(data_.get()); return reinterpret_cast<T*>(data_.get());
} }
template <typename T = char, typename internal::ByteType<T>::t = 0> template <typename T = uint8_t, typename internal::ByteType<T>::t = 0>
T* data() { T* data() {
assert(IsConsistent()); assert(IsConsistent());
return reinterpret_cast<T*>(data_.get()); return reinterpret_cast<T*>(data_.get());
@ -133,7 +118,7 @@ class Buffer final {
// Replace the contents of the buffer. Accepts the same types as the // Replace the contents of the buffer. Accepts the same types as the
// constructors. // constructors.
template <typename T, typename internal::ByteTypeOrVoid<T>::t = 0> template <typename T, typename internal::ByteType<T>::t = 0>
void SetData(const T* data, size_t size) { void SetData(const T* data, size_t size) {
assert(IsConsistent()); assert(IsConsistent());
size_ = 0; size_ = 0;
@ -146,7 +131,7 @@ class Buffer final {
void SetData(const Buffer& buf) { SetData(buf.data(), buf.size()); } void SetData(const Buffer& buf) { SetData(buf.data(), buf.size()); }
// Append data to the buffer. Accepts the same types as the constructors. // Append data to the buffer. Accepts the same types as the constructors.
template <typename T, typename internal::ByteTypeOrVoid<T>::t = 0> template <typename T, typename internal::ByteType<T>::t = 0>
void AppendData(const T* data, size_t size) { void AppendData(const T* data, size_t size) {
assert(IsConsistent()); assert(IsConsistent());
const size_t new_size = size_ + size; const size_t new_size = size_ + size;

View File

@ -137,11 +137,11 @@ TEST(BufferTest, TestEnsureCapacityLarger) {
TEST(BufferTest, TestMoveConstruct) { TEST(BufferTest, TestMoveConstruct) {
Buffer buf1(kTestData, 3, 40); Buffer buf1(kTestData, 3, 40);
const uint8_t* data = buf1.data<uint8_t>(); const uint8_t* data = buf1.data();
Buffer buf2(buf1.Pass()); Buffer buf2(buf1.Pass());
EXPECT_EQ(buf2.size(), 3u); EXPECT_EQ(buf2.size(), 3u);
EXPECT_EQ(buf2.capacity(), 40u); EXPECT_EQ(buf2.capacity(), 40u);
EXPECT_EQ(buf2.data<uint8_t>(), data); EXPECT_EQ(buf2.data(), data);
buf1.Clear(); buf1.Clear();
EXPECT_EQ(buf1.size(), 0u); EXPECT_EQ(buf1.size(), 0u);
EXPECT_EQ(buf1.capacity(), 0u); EXPECT_EQ(buf1.capacity(), 0u);
@ -150,12 +150,12 @@ TEST(BufferTest, TestMoveConstruct) {
TEST(BufferTest, TestMoveAssign) { TEST(BufferTest, TestMoveAssign) {
Buffer buf1(kTestData, 3, 40); Buffer buf1(kTestData, 3, 40);
const uint8_t* data = buf1.data<uint8_t>(); const uint8_t* data = buf1.data();
Buffer buf2(kTestData); Buffer buf2(kTestData);
buf2 = buf1.Pass(); buf2 = buf1.Pass();
EXPECT_EQ(buf2.size(), 3u); EXPECT_EQ(buf2.size(), 3u);
EXPECT_EQ(buf2.capacity(), 40u); EXPECT_EQ(buf2.capacity(), 40u);
EXPECT_EQ(buf2.data<uint8_t>(), data); EXPECT_EQ(buf2.data(), data);
buf1.Clear(); buf1.Clear();
EXPECT_EQ(buf1.size(), 0u); EXPECT_EQ(buf1.size(), 0u);
EXPECT_EQ(buf1.capacity(), 0u); EXPECT_EQ(buf1.capacity(), 0u);
@ -165,16 +165,16 @@ TEST(BufferTest, TestMoveAssign) {
TEST(BufferTest, TestSwap) { TEST(BufferTest, TestSwap) {
Buffer buf1(kTestData, 3); Buffer buf1(kTestData, 3);
Buffer buf2(kTestData, 6, 40); Buffer buf2(kTestData, 6, 40);
uint8_t* data1 = buf1.data<uint8_t>(); uint8_t* data1 = buf1.data();
uint8_t* data2 = buf2.data<uint8_t>(); uint8_t* data2 = buf2.data();
using std::swap; using std::swap;
swap(buf1, buf2); swap(buf1, buf2);
EXPECT_EQ(buf1.size(), 6u); EXPECT_EQ(buf1.size(), 6u);
EXPECT_EQ(buf1.capacity(), 40u); EXPECT_EQ(buf1.capacity(), 40u);
EXPECT_EQ(buf1.data<uint8_t>(), data2); EXPECT_EQ(buf1.data(), data2);
EXPECT_EQ(buf2.size(), 3u); EXPECT_EQ(buf2.size(), 3u);
EXPECT_EQ(buf2.capacity(), 3u); EXPECT_EQ(buf2.capacity(), 3u);
EXPECT_EQ(buf2.data<uint8_t>(), data1); EXPECT_EQ(buf2.data(), data1);
} }
} // namespace rtc } // namespace rtc

View File

@ -82,7 +82,7 @@ class LoopbackTransportTest : public webrtc::Transport {
packets_sent_++; packets_sent_++;
rtc::Buffer* buffer = rtc::Buffer* buffer =
new rtc::Buffer(reinterpret_cast<const uint8_t*>(data), len); new rtc::Buffer(reinterpret_cast<const uint8_t*>(data), len);
last_sent_packet_ = buffer->data<uint8_t>(); last_sent_packet_ = buffer->data();
last_sent_packet_len_ = len; last_sent_packet_len_ = len;
total_bytes_sent_ += len; total_bytes_sent_ += len;
sent_packets_.push_back(buffer); sent_packets_.push_back(buffer);