From 29159ca979cb6670dd02cc09a45df1404f006793 Mon Sep 17 00:00:00 2001 From: Victor Boivie Date: Mon, 10 Jan 2022 12:52:13 +0100 Subject: [PATCH] dcsctp: Use c++17 structured bindings As WebRTC now supports C++17, simplify the code of dcSCTP by binding return values from std::pair or std::tuple to separate names. Bug: webrtc:13220 Change-Id: Ie49154ff4c823e1528deaef7e372cbc550923bc2 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/246442 Reviewed-by: Danil Chapovalov Commit-Queue: Victor Boivie Cr-Commit-Position: refs/heads/main@{#35773} --- net/dcsctp/rx/reassembly_queue.cc | 4 +- .../rx/traditional_reassembly_streams.cc | 36 ++++++------- .../socket/transmission_control_block.cc | 4 +- net/dcsctp/tx/outstanding_data.cc | 54 +++++++------------ net/dcsctp/tx/rr_send_queue.cc | 28 +++++----- 5 files changed, 51 insertions(+), 75 deletions(-) diff --git a/net/dcsctp/rx/reassembly_queue.cc b/net/dcsctp/rx/reassembly_queue.cc index 36ade9230a..5791d6805c 100644 --- a/net/dcsctp/rx/reassembly_queue.cc +++ b/net/dcsctp/rx/reassembly_queue.cc @@ -175,9 +175,7 @@ bool ReassemblyQueue::MaybeResetStreamsDeferred(TSN cum_ack_tsn) { // https://tools.ietf.org/html/rfc6525#section-5.2.2 // "Any queued TSNs (queued at step E2) MUST now be released and processed // normally." - for (auto& p : deferred_chunks) { - const TSN& tsn = p.first; - Data& data = p.second; + for (auto& [tsn, data] : deferred_chunks) { queued_bytes_ -= data.size(); Add(tsn, std::move(data)); } diff --git a/net/dcsctp/rx/traditional_reassembly_streams.cc b/net/dcsctp/rx/traditional_reassembly_streams.cc index d004824354..4ccc2e5278 100644 --- a/net/dcsctp/rx/traditional_reassembly_streams.cc +++ b/net/dcsctp/rx/traditional_reassembly_streams.cc @@ -105,12 +105,12 @@ TraditionalReassemblyStreams::TraditionalReassemblyStreams( int TraditionalReassemblyStreams::UnorderedStream::Add(UnwrappedTSN tsn, Data data) { int queued_bytes = data.size(); - auto p = chunks_.emplace(tsn, std::move(data)); - if (!p.second /* !inserted */) { + auto [it, inserted] = chunks_.emplace(tsn, std::move(data)); + if (!inserted) { return 0; } - queued_bytes -= TryToAssembleMessage(p.first); + queued_bytes -= TryToAssembleMessage(it); return queued_bytes; } @@ -225,8 +225,8 @@ int TraditionalReassemblyStreams::OrderedStream::Add(UnwrappedTSN tsn, int queued_bytes = data.size(); UnwrappedSSN ssn = ssn_unwrapper_.Unwrap(data.ssn); - auto p = chunks_by_ssn_[ssn].emplace(tsn, std::move(data)); - if (!p.second /* !inserted */) { + auto [unused, inserted] = chunks_by_ssn_[ssn].emplace(tsn, std::move(data)); + if (!inserted) { return 0; } @@ -275,8 +275,8 @@ size_t TraditionalReassemblyStreams::HandleForwardTsn( size_t bytes_removed = 0; // The `skipped_streams` only cover ordered messages - need to // iterate all unordered streams manually to remove those chunks. - for (auto& entry : unordered_streams_) { - bytes_removed += entry.second.EraseTo(new_cumulative_ack_tsn); + for (auto& [unused, stream] : unordered_streams_) { + bytes_removed += stream.EraseTo(new_cumulative_ack_tsn); } for (const auto& skipped_stream : skipped_streams) { @@ -292,9 +292,7 @@ size_t TraditionalReassemblyStreams::HandleForwardTsn( void TraditionalReassemblyStreams::ResetStreams( rtc::ArrayView stream_ids) { if (stream_ids.empty()) { - for (auto& entry : ordered_streams_) { - const StreamID& stream_id = entry.first; - OrderedStream& stream = entry.second; + for (auto& [stream_id, stream] : ordered_streams_) { RTC_DLOG(LS_VERBOSE) << log_prefix_ << "Resetting implicit stream_id=" << *stream_id; stream.Reset(); @@ -314,14 +312,14 @@ void TraditionalReassemblyStreams::ResetStreams( HandoverReadinessStatus TraditionalReassemblyStreams::GetHandoverReadiness() const { HandoverReadinessStatus status; - for (const auto& entry : ordered_streams_) { - if (entry.second.has_unassembled_chunks()) { + for (const auto& [unused, stream] : ordered_streams_) { + if (stream.has_unassembled_chunks()) { status.Add(HandoverUnreadinessReason::kOrderedStreamHasUnassembledChunks); break; } } - for (const auto& entry : unordered_streams_) { - if (entry.second.has_unassembled_chunks()) { + for (const auto& [unused, stream] : unordered_streams_) { + if (stream.has_unassembled_chunks()) { status.Add( HandoverUnreadinessReason::kUnorderedStreamHasUnassembledChunks); break; @@ -332,15 +330,15 @@ HandoverReadinessStatus TraditionalReassemblyStreams::GetHandoverReadiness() void TraditionalReassemblyStreams::AddHandoverState( DcSctpSocketHandoverState& state) { - for (const auto& entry : ordered_streams_) { + for (const auto& [stream_id, stream] : ordered_streams_) { DcSctpSocketHandoverState::OrderedStream state_stream; - state_stream.id = entry.first.value(); - state_stream.next_ssn = entry.second.next_ssn().value(); + state_stream.id = stream_id.value(); + state_stream.next_ssn = stream.next_ssn().value(); state.rx.ordered_streams.push_back(std::move(state_stream)); } - for (const auto& entry : unordered_streams_) { + for (const auto& [stream_id, unused] : unordered_streams_) { DcSctpSocketHandoverState::UnorderedStream state_stream; - state_stream.id = entry.first.value(); + state_stream.id = stream_id.value(); state.rx.unordered_streams.push_back(std::move(state_stream)); } } diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc index 2e4e968737..0fc0d4c029 100644 --- a/net/dcsctp/socket/transmission_control_block.cc +++ b/net/dcsctp/socket/transmission_control_block.cc @@ -140,9 +140,7 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder, auto chunks = retransmission_queue_.GetChunksToSend(now, builder.bytes_remaining()); - for (auto& elem : chunks) { - TSN tsn = elem.first; - Data data = std::move(elem.second); + for (auto& [tsn, data] : chunks) { if (capabilities_.message_interleaving) { builder.Add(IDataChunk(tsn, std::move(data), false)); } else { diff --git a/net/dcsctp/tx/outstanding_data.cc b/net/dcsctp/tx/outstanding_data.cc index dc998def2f..05277d0584 100644 --- a/net/dcsctp/tx/outstanding_data.cc +++ b/net/dcsctp/tx/outstanding_data.cc @@ -74,14 +74,14 @@ bool OutstandingData::IsConsistent() const { size_t actual_outstanding_items = 0; std::set actual_to_be_retransmitted; - for (const auto& elem : outstanding_data_) { - if (elem.second.is_outstanding()) { - actual_outstanding_bytes += GetSerializedChunkSize(elem.second.data()); + for (const auto& [tsn, item] : outstanding_data_) { + if (item.is_outstanding()) { + actual_outstanding_bytes += GetSerializedChunkSize(item.data()); ++actual_outstanding_items; } - if (elem.second.should_be_retransmitted()) { - actual_to_be_retransmitted.insert(elem.first); + if (item.should_be_retransmitted()) { + actual_to_be_retransmitted.insert(tsn); } } @@ -263,10 +263,7 @@ void OutstandingData::AbandonAllFor(const Item& item) { << *tsn.Wrap(); } - for (auto& elem : outstanding_data_) { - UnwrappedTSN tsn = elem.first; - Item& other = elem.second; - + for (auto& [tsn, other] : outstanding_data_) { if (!other.is_abandoned() && other.data().stream_id == item.data().stream_id && other.data().is_unordered == item.data().is_unordered && @@ -318,10 +315,7 @@ std::vector> OutstandingData::GetChunksToBeRetransmitted( } void OutstandingData::ExpireOutstandingChunks(TimeMs now) { - for (const auto& elem : outstanding_data_) { - UnwrappedTSN tsn = elem.first; - const Item& item = elem.second; - + for (const auto& [tsn, item] : outstanding_data_) { // Chunks that are nacked can be expired. Care should be taken not to expire // unacked (in-flight) chunks as they might have been received, but the SACK // is either delayed or in-flight and may be received later. @@ -378,9 +372,7 @@ absl::optional OutstandingData::Insert( } void OutstandingData::NackAll() { - for (auto& elem : outstanding_data_) { - UnwrappedTSN tsn = elem.first; - Item& item = elem.second; + for (auto& [tsn, item] : outstanding_data_) { if (!item.is_acked()) { NackItem(tsn, item, /*retransmit_now=*/true); } @@ -406,21 +398,21 @@ std::vector> OutstandingData::GetChunkStatesForTesting() const { std::vector> states; states.emplace_back(last_cumulative_tsn_ack_.Wrap(), State::kAcked); - for (const auto& elem : outstanding_data_) { + for (const auto& [tsn, item] : outstanding_data_) { State state; - if (elem.second.is_abandoned()) { + if (item.is_abandoned()) { state = State::kAbandoned; - } else if (elem.second.should_be_retransmitted()) { + } else if (item.should_be_retransmitted()) { state = State::kToBeRetransmitted; - } else if (elem.second.is_acked()) { + } else if (item.is_acked()) { state = State::kAcked; - } else if (elem.second.is_outstanding()) { + } else if (item.is_outstanding()) { state = State::kInFlight; } else { state = State::kNacked; } - states.emplace_back(elem.first.Wrap(), state); + states.emplace_back(tsn.Wrap(), state); } return states; } @@ -438,10 +430,7 @@ ForwardTsnChunk OutstandingData::CreateForwardTsn() const { std::map skipped_per_ordered_stream; UnwrappedTSN new_cumulative_ack = last_cumulative_tsn_ack_; - for (const auto& elem : outstanding_data_) { - UnwrappedTSN tsn = elem.first; - const Item& item = elem.second; - + for (const auto& [tsn, item] : outstanding_data_) { if ((tsn != new_cumulative_ack.next_value()) || !item.is_abandoned()) { break; } @@ -454,8 +443,8 @@ ForwardTsnChunk OutstandingData::CreateForwardTsn() const { std::vector skipped_streams; skipped_streams.reserve(skipped_per_ordered_stream.size()); - for (const auto& elem : skipped_per_ordered_stream) { - skipped_streams.emplace_back(elem.first, elem.second); + for (const auto& [stream_id, ssn] : skipped_per_ordered_stream) { + skipped_streams.emplace_back(stream_id, ssn); } return ForwardTsnChunk(new_cumulative_ack.Wrap(), std::move(skipped_streams)); } @@ -464,10 +453,7 @@ IForwardTsnChunk OutstandingData::CreateIForwardTsn() const { std::map, MID> skipped_per_stream; UnwrappedTSN new_cumulative_ack = last_cumulative_tsn_ack_; - for (const auto& elem : outstanding_data_) { - UnwrappedTSN tsn = elem.first; - const Item& item = elem.second; - + for (const auto& [tsn, item] : outstanding_data_) { if ((tsn != new_cumulative_ack.next_value()) || !item.is_abandoned()) { break; } @@ -482,9 +468,7 @@ IForwardTsnChunk OutstandingData::CreateIForwardTsn() const { std::vector skipped_streams; skipped_streams.reserve(skipped_per_stream.size()); - for (const auto& elem : skipped_per_stream) { - const std::pair& stream = elem.first; - MID message_id = elem.second; + for (const auto& [stream, message_id] : skipped_per_stream) { skipped_streams.emplace_back(stream.first, stream.second, message_id); } diff --git a/net/dcsctp/tx/rr_send_queue.cc b/net/dcsctp/tx/rr_send_queue.cc index 21744cc0a0..a20ccb2448 100644 --- a/net/dcsctp/tx/rr_send_queue.cc +++ b/net/dcsctp/tx/rr_send_queue.cc @@ -76,8 +76,8 @@ void RRSendQueue::OutgoingStream::AddHandoverState( bool RRSendQueue::IsConsistent() const { size_t total_buffered_amount = 0; - for (const auto& stream_entry : streams_) { - total_buffered_amount += stream_entry.second.buffered_amount().value(); + for (const auto& [unused, stream] : streams_) { + total_buffered_amount += stream.buffered_amount().value(); } if (previous_message_has_ended_) { @@ -391,9 +391,8 @@ void RRSendQueue::PrepareResetStreams(rtc::ArrayView streams) { bool RRSendQueue::CanResetStreams() const { // Streams can be reset if those streams that are paused don't have any // messages that are partially sent. - for (auto& stream : streams_) { - if (stream.second.is_paused() && - stream.second.has_partially_sent_message()) { + for (auto& [unused, stream] : streams_) { + if (stream.is_paused() && stream.has_partially_sent_message()) { return false; } } @@ -401,17 +400,17 @@ bool RRSendQueue::CanResetStreams() const { } void RRSendQueue::CommitResetStreams() { - for (auto& stream_entry : streams_) { - if (stream_entry.second.is_paused()) { - stream_entry.second.Reset(); + for (auto& [unused, stream] : streams_) { + if (stream.is_paused()) { + stream.Reset(); } } RTC_DCHECK(IsConsistent()); } void RRSendQueue::RollbackResetStreams() { - for (auto& stream_entry : streams_) { - stream_entry.second.Resume(); + for (auto& [unused, stream] : streams_) { + stream.Resume(); } RTC_DCHECK(IsConsistent()); } @@ -419,8 +418,7 @@ void RRSendQueue::RollbackResetStreams() { void RRSendQueue::Reset() { // Recalculate buffered amount, as partially sent messages may have been put // fully back in the queue. - for (auto& stream_entry : streams_) { - OutgoingStream& stream = stream_entry.second; + for (auto& [unused, stream] : streams_) { stream.Reset(); } previous_message_has_ended_ = true; @@ -471,10 +469,10 @@ HandoverReadinessStatus RRSendQueue::GetHandoverReadiness() const { } void RRSendQueue::AddHandoverState(DcSctpSocketHandoverState& state) { - for (const auto& entry : streams_) { + for (const auto& [stream_id, stream] : streams_) { DcSctpSocketHandoverState::OutgoingStream state_stream; - state_stream.id = entry.first.value(); - entry.second.AddHandoverState(state_stream); + state_stream.id = stream_id.value(); + stream.AddHandoverState(state_stream); state.tx.streams.push_back(std::move(state_stream)); } }