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(