Add a proper API to create memslices from the QUIC code.
Current API provides an (allocator, size) constructor. However, this just creates a buffer with random data in it and no way to change it, since memslices are read-only. Current code uses const_cast; this CL creates a proper API and updates all existing call sites to use it.
gfe-relnote: n/a (no functional change)
PiperOrigin-RevId: 287072774
Change-Id: Ie4dd2eae6db2ec91b087f5d41887ea3948ee411a
diff --git a/quic/core/frames/quic_frame.cc b/quic/core/frames/quic_frame.cc
index e640179..44d3cc8 100644
--- a/quic/core/frames/quic_frame.cc
+++ b/quic/core/frames/quic_frame.cc
@@ -4,6 +4,7 @@
#include "net/third_party/quiche/src/quic/core/frames/quic_frame.h"
+#include "net/third_party/quiche/src/quic/core/quic_buffer_allocator.h"
#include "net/third_party/quiche/src/quic/core/quic_constants.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
@@ -319,10 +320,11 @@
copy.message_frame->data = frame.message_frame->data;
copy.message_frame->message_length = frame.message_frame->message_length;
for (const auto& slice : frame.message_frame->message_data) {
- QuicMemSlice copy_slice(allocator, slice.length());
- memcpy(const_cast<char*>(copy_slice.data()), slice.data(),
- slice.length());
- copy.message_frame->message_data.push_back(std::move(copy_slice));
+ QuicUniqueBufferPtr buffer =
+ MakeUniqueBuffer(allocator, slice.length());
+ memcpy(buffer.get(), slice.data(), slice.length());
+ copy.message_frame->message_data.push_back(
+ QuicMemSlice(std::move(buffer), slice.length()));
}
break;
case NEW_TOKEN_FRAME:
diff --git a/quic/core/quic_buffer_allocator.h b/quic/core/quic_buffer_allocator.h
index 10df369..4b0abf4 100644
--- a/quic/core/quic_buffer_allocator.h
+++ b/quic/core/quic_buffer_allocator.h
@@ -7,6 +7,8 @@
#include <stddef.h>
+#include <memory>
+
#include "net/third_party/quiche/src/quic/platform/api/quic_export.h"
namespace quic {
@@ -32,6 +34,28 @@
virtual void MarkAllocatorIdle() {}
};
+// A deleter that can be used to manage ownership of buffers allocated via
+// QuicBufferAllocator through std::unique_ptr.
+class QUIC_EXPORT_PRIVATE QuicBufferDeleter {
+ public:
+ explicit QuicBufferDeleter(QuicBufferAllocator* allocator)
+ : allocator_(allocator) {}
+
+ QuicBufferAllocator* allocator() { return allocator_; }
+ void operator()(char* buffer) { allocator_->Delete(buffer); }
+
+ private:
+ QuicBufferAllocator* allocator_;
+};
+
+using QuicUniqueBufferPtr = std::unique_ptr<char[], QuicBufferDeleter>;
+
+inline QuicUniqueBufferPtr MakeUniqueBuffer(QuicBufferAllocator* allocator,
+ size_t size) {
+ return QuicUniqueBufferPtr(allocator->New(size),
+ QuicBufferDeleter(allocator));
+}
+
} // namespace quic
#endif // QUICHE_QUIC_CORE_QUIC_BUFFER_ALLOCATOR_H_
diff --git a/quic/core/quic_stream_send_buffer.cc b/quic/core/quic_stream_send_buffer.cc
index 2008cf2..d91ceb8 100644
--- a/quic/core/quic_stream_send_buffer.cc
+++ b/quic/core/quic_stream_send_buffer.cc
@@ -67,10 +67,10 @@
GetQuicFlag(FLAGS_quic_send_buffer_max_data_slice_size);
while (data_length > 0) {
size_t slice_len = std::min(data_length, max_data_slice_size);
- QuicMemSlice slice(allocator_, slice_len);
+ QuicUniqueBufferPtr buffer = MakeUniqueBuffer(allocator_, slice_len);
QuicUtils::CopyToBuffer(iov, iov_count, iov_offset, slice_len,
- const_cast<char*>(slice.data()));
- SaveMemSlice(std::move(slice));
+ buffer.get());
+ SaveMemSlice(QuicMemSlice(std::move(buffer), slice_len));
data_length -= slice_len;
iov_offset += slice_len;
}
diff --git a/quic/core/quic_stream_send_buffer_test.cc b/quic/core/quic_stream_send_buffer_test.cc
index 8f864c6..325dca0 100644
--- a/quic/core/quic_stream_send_buffer_test.cc
+++ b/quic/core/quic_stream_send_buffer_test.cc
@@ -39,10 +39,12 @@
iov[0] = MakeIovec(quiche::QuicheStringPiece(data1));
iov[1] = MakeIovec(quiche::QuicheStringPiece(data2));
- QuicMemSlice slice1(&allocator_, 1024);
- memset(const_cast<char*>(slice1.data()), 'c', 1024);
- QuicMemSlice slice2(&allocator_, 768);
- memset(const_cast<char*>(slice2.data()), 'd', 768);
+ QuicUniqueBufferPtr buffer1 = MakeUniqueBuffer(&allocator_, 1024);
+ memset(buffer1.get(), 'c', 1024);
+ QuicMemSlice slice1(std::move(buffer1), 1024);
+ QuicUniqueBufferPtr buffer2 = MakeUniqueBuffer(&allocator_, 768);
+ memset(buffer2.get(), 'd', 768);
+ QuicMemSlice slice2(std::move(buffer2), 768);
if (!GetQuicReloadableFlag(quic_interval_deque)) {
// Index starts from not pointing to any slice.
@@ -362,8 +364,9 @@
// Last offset is end offset of last slice.
EXPECT_EQ(3840u, QuicStreamSendBufferPeer::EndOffset(&send_buffer_));
}
- QuicMemSlice slice(&allocator_, 60);
- memset(const_cast<char*>(slice.data()), 'e', 60);
+ QuicUniqueBufferPtr buffer = MakeUniqueBuffer(&allocator_, 60);
+ memset(buffer.get(), 'e', 60);
+ QuicMemSlice slice(std::move(buffer), 60);
send_buffer_.SaveMemSlice(std::move(slice));
if (!GetQuicReloadableFlag(quic_interval_deque)) {
// With new data, index points to the new data.
diff --git a/quic/masque/masque_compression_engine.cc b/quic/masque/masque_compression_engine.cc
index 804a8cf..dbc4580 100644
--- a/quic/masque/masque_compression_engine.cc
+++ b/quic/masque/masque_compression_engine.cc
@@ -6,6 +6,7 @@
#include <cstdint>
+#include "net/third_party/quiche/src/quic/core/quic_buffer_allocator.h"
#include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
#include "net/third_party/quiche/src/quic/core/quic_framer.h"
@@ -102,33 +103,32 @@
uint8_t first_byte,
bool long_header,
QuicDataReader* reader,
- QuicMemSlice* slice) {
- QuicDataWriter writer(slice->length(), const_cast<char*>(slice->data()));
+ QuicDataWriter* writer) {
if (validated) {
QUIC_DVLOG(1) << "Compressing using validated flow_id " << flow_id;
- if (!writer.WriteVarInt62(flow_id)) {
+ if (!writer->WriteVarInt62(flow_id)) {
QUIC_BUG << "Failed to write flow_id";
return false;
}
} else {
QUIC_DVLOG(1) << "Compressing using unvalidated flow_id " << flow_id;
- if (!writer.WriteVarInt62(kFlowId0)) {
+ if (!writer->WriteVarInt62(kFlowId0)) {
QUIC_BUG << "Failed to write kFlowId0";
return false;
}
- if (!writer.WriteVarInt62(flow_id)) {
+ if (!writer->WriteVarInt62(flow_id)) {
QUIC_BUG << "Failed to write flow_id";
return false;
}
- if (!writer.WriteLengthPrefixedConnectionId(client_connection_id)) {
+ if (!writer->WriteLengthPrefixedConnectionId(client_connection_id)) {
QUIC_BUG << "Failed to write client_connection_id";
return false;
}
- if (!writer.WriteLengthPrefixedConnectionId(server_connection_id)) {
+ if (!writer->WriteLengthPrefixedConnectionId(server_connection_id)) {
QUIC_BUG << "Failed to write server_connection_id";
return false;
}
- if (!writer.WriteUInt16(server_address.port())) {
+ if (!writer->WriteUInt16(server_address.port())) {
QUIC_BUG << "Failed to write port";
return false;
}
@@ -153,16 +153,16 @@
QUIC_BUG << "Unexpected server_address " << server_address;
return false;
}
- if (!writer.WriteUInt8(address_id)) {
+ if (!writer->WriteUInt8(address_id)) {
QUIC_BUG << "Failed to write address_id";
return false;
}
- if (!writer.WriteStringPiece(peer_ip_bytes)) {
+ if (!writer->WriteStringPiece(peer_ip_bytes)) {
QUIC_BUG << "Failed to write IP address";
return false;
}
}
- if (!writer.WriteUInt8(first_byte)) {
+ if (!writer->WriteUInt8(first_byte)) {
QUIC_BUG << "Failed to write first_byte";
return false;
}
@@ -172,7 +172,7 @@
QUIC_DLOG(ERROR) << "Failed to read version";
return false;
}
- if (!writer.WriteUInt32(version_label)) {
+ if (!writer->WriteUInt32(version_label)) {
QUIC_BUG << "Failed to write version";
return false;
}
@@ -214,7 +214,7 @@
}
}
quiche::QuicheStringPiece packet_payload = reader->ReadRemainingPayload();
- if (!writer.WriteStringPiece(packet_payload)) {
+ if (!writer->WriteStringPiece(packet_payload)) {
QUIC_BUG << "Failed to write packet_payload";
return false;
}
@@ -276,17 +276,19 @@
sizeof(server_address.port()) + sizeof(uint8_t) +
server_address.host().ToPackedString().length();
}
- QuicMemSlice slice(
+ QuicUniqueBufferPtr buffer = MakeUniqueBuffer(
masque_session_->connection()->helper()->GetStreamSendBufferAllocator(),
slice_length);
+ QuicDataWriter writer(slice_length, buffer.get());
- if (!WriteCompressedPacketToSlice(client_connection_id, server_connection_id,
- server_address, destination_connection_id,
- source_connection_id, flow_id, validated,
- first_byte, long_header, &reader, &slice)) {
+ if (!WriteCompressedPacketToSlice(
+ client_connection_id, server_connection_id, server_address,
+ destination_connection_id, source_connection_id, flow_id, validated,
+ first_byte, long_header, &reader, &writer)) {
return;
}
+ QuicMemSlice slice(std::move(buffer), slice_length);
MessageResult message_result =
masque_session_->SendMessage(QuicMemSliceSpan(&slice));
diff --git a/quic/masque/masque_compression_engine.h b/quic/masque/masque_compression_engine.h
index 32ffc9d..95328df 100644
--- a/quic/masque/masque_compression_engine.h
+++ b/quic/masque/masque_compression_engine.h
@@ -104,7 +104,7 @@
uint8_t first_byte,
bool long_header,
QuicDataReader* reader,
- QuicMemSlice* slice);
+ QuicDataWriter* writer);
// Parses compression context from flow ID 0 during decompression.
bool ParseCompressionContext(QuicDataReader* reader,
diff --git a/quic/platform/api/quic_mem_slice.h b/quic/platform/api/quic_mem_slice.h
index a40d638..2d44085 100644
--- a/quic/platform/api/quic_mem_slice.h
+++ b/quic/platform/api/quic_mem_slice.h
@@ -26,14 +26,12 @@
// Constructs a empty QuicMemSlice with no underlying data and 0 reference
// count.
QuicMemSlice() = default;
- // Let |allocator| allocate a data buffer of |length|, then construct
- // QuicMemSlice with reference count 1 from the allocated data buffer.
- // Once all of the references to the allocated data buffer are released,
- // |allocator| is responsible to free the memory. |allocator| must
- // not be null, and |length| must not be 0. To construct an empty
- // QuicMemSlice, use the zero-argument constructor instead.
- QuicMemSlice(QuicBufferAllocator* allocator, size_t length)
- : impl_(allocator, length) {}
+
+ // Constructs a QuicMemSlice that takes ownership of |buffer|. |length| must
+ // not be zero. To construct an empty QuicMemSlice, use the zero-argument
+ // constructor instead.
+ QuicMemSlice(QuicUniqueBufferPtr buffer, size_t length)
+ : impl_(std::move(buffer), length) {}
// Constructs QuicMemSlice from |impl|. It takes the reference away from
// |impl|.
diff --git a/quic/platform/api/quic_mem_slice_test.cc b/quic/platform/api/quic_mem_slice_test.cc
index 185f536..d441afa 100644
--- a/quic/platform/api/quic_mem_slice_test.cc
+++ b/quic/platform/api/quic_mem_slice_test.cc
@@ -15,7 +15,7 @@
public:
QuicMemSliceTest() {
size_t length = 1024;
- slice_ = QuicMemSlice(&allocator_, length);
+ slice_ = QuicMemSlice(MakeUniqueBuffer(&allocator_, length), length);
orig_data_ = slice_.data();
orig_length_ = slice_.length();
}
diff --git a/quic/qbone/qbone_session_base.cc b/quic/qbone/qbone_session_base.cc
index e3fbb6e..c59f412 100644
--- a/quic/qbone/qbone_session_base.cc
+++ b/quic/qbone/qbone_session_base.cc
@@ -9,6 +9,7 @@
#include <utility>
+#include "net/third_party/quiche/src/quic/core/quic_buffer_allocator.h"
#include "net/third_party/quiche/src/quic/core/quic_data_reader.h"
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_exported_stats.h"
@@ -139,9 +140,10 @@
}
if (send_packets_as_messages_) {
- QuicMemSlice slice(connection()->helper()->GetStreamSendBufferAllocator(),
- packet.size());
- memcpy(const_cast<char*>(slice.data()), packet.data(), packet.size());
+ QuicUniqueBufferPtr buffer = MakeUniqueBuffer(
+ connection()->helper()->GetStreamSendBufferAllocator(), packet.size());
+ memcpy(buffer.get(), packet.data(), packet.size());
+ QuicMemSlice slice(std::move(buffer), packet.size());
switch (SendMessage(QuicMemSliceSpan(&slice), /*flush=*/true).status) {
case MESSAGE_STATUS_SUCCESS:
break;
diff --git a/quic/quartc/quartc_multiplexer.cc b/quic/quartc/quartc_multiplexer.cc
index 3abc3cd..376fac0 100644
--- a/quic/quartc/quartc_multiplexer.cc
+++ b/quic/quartc/quartc_multiplexer.cc
@@ -7,6 +7,7 @@
#include <cstdint>
#include <utility>
+#include "net/third_party/quiche/src/quic/core/quic_buffer_allocator.h"
#include "net/third_party/quiche/src/quic/core/quic_data_writer.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice_span.h"
@@ -95,10 +96,10 @@
}
QuicMemSlice QuartcSendChannel::EncodeChannelId() {
- QuicMemSlice id_slice(allocator_, encoded_length_);
- QuicDataWriter writer(encoded_length_, const_cast<char*>(id_slice.data()));
+ QuicUniqueBufferPtr buffer = MakeUniqueBuffer(allocator_, encoded_length_);
+ QuicDataWriter writer(encoded_length_, buffer.get());
writer.WriteVarInt62(id_);
- return id_slice;
+ return QuicMemSlice(std::move(buffer), encoded_length_);
}
QuartcMultiplexer::QuartcMultiplexer(
