Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop using C++ auto to assign results of arithmetic operations #1386

Merged
merged 2 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### NEXT

- Worker: Fix possible value overflow in `FeedbackRtpTransport.cpp` ([PR #1386](https://github.com/versatica/mediasoup/pull/1386), credits to @Lynnworld).

### 3.14.2

- Update worker subprojects ([PR #1376](https://github.com/versatica/mediasoup/pull/1376)).
Expand Down
4 changes: 3 additions & 1 deletion worker/src/RTC/RTCP/FeedbackRtpTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,9 @@ namespace RTC

// Check if there are too many missing packets.
{
auto missingPackets = sequenceNumber - (this->latestSequenceNumber + 1);
// NOTE: We CANNOT use auto here, we must use uint16_t. Otherwise this is a bug.
// https://github.com/versatica/mediasoup/issues/1385#issuecomment-2084982087
const uint16_t missingPackets = sequenceNumber - (this->latestSequenceNumber + 1);

if (missingPackets > FeedbackRtpTransportPacket::maxMissingPackets)
{
Expand Down
19 changes: 9 additions & 10 deletions worker/src/RTC/RtpRetransmissionBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ namespace RTC
return nullptr;
}

const auto idx = static_cast<uint16_t>(seq - oldestItem->sequenceNumber);
const uint16_t idx = seq - oldestItem->sequenceNumber;

if (idx > static_cast<uint16_t>(this->buffer.size() - 1))
if (static_cast<size_t>(idx) > this->buffer.size() - 1)
{
return nullptr;
}
Expand Down Expand Up @@ -198,14 +198,13 @@ namespace RTC

// Calculate how many blank slots it would be necessary to add when
// pushing new item to the back of the buffer.
auto numBlankSlots = static_cast<uint16_t>(seq - newestItem->sequenceNumber - 1);
uint16_t numBlankSlots = seq - newestItem->sequenceNumber - 1;

// We may have to remove oldest items not to exceed the maximum size of
// the buffer.
if (this->buffer.size() + numBlankSlots + 1 > this->maxItems)
{
const auto numItemsToRemove =
static_cast<uint16_t>(this->buffer.size() + numBlankSlots + 1 - this->maxItems);
const uint16_t numItemsToRemove = this->buffer.size() + numBlankSlots + 1 - this->maxItems;

// If num of items to be removed exceed buffer size minus one (needed to
// allocate current packet) then we must clear the entire buffer.
Expand Down Expand Up @@ -286,7 +285,7 @@ namespace RTC

// Calculate how many blank slots it would be necessary to add when
// pushing new item to the fton of the buffer.
const auto numBlankSlots = static_cast<uint16_t>(oldestItem->sequenceNumber - seq - 1);
const uint16_t numBlankSlots = oldestItem->sequenceNumber - seq - 1;

// If adding this packet (and needed blank slots) to the front makes the
// buffer exceed its max size, discard this packet.
Expand Down Expand Up @@ -339,11 +338,11 @@ namespace RTC
}

// idx is the intended position of the received packet in the buffer.
const auto idx = static_cast<uint16_t>(seq - oldestItem->sequenceNumber);
const uint16_t idx = seq - oldestItem->sequenceNumber;

// Validate that packet timestamp is equal or higher than the timestamp of
// the immediate older packet (if any).
for (auto idx2 = static_cast<int32_t>(idx - 1); idx2 >= 0; --idx2)
for (int32_t idx2 = idx - 1; idx2 >= 0; --idx2)
{
const auto* olderItem = this->buffer.at(idx2);

Expand Down Expand Up @@ -374,7 +373,7 @@ namespace RTC

// Validate that packet timestamp is equal or less than the timestamp of
// the immediate newer packet (if any).
for (auto idx2 = static_cast<size_t>(idx + 1); idx2 < this->buffer.size(); ++idx2)
for (size_t idx2 = idx + 1; idx2 < this->buffer.size(); ++idx2)
{
const auto* newerItem = this->buffer.at(idx2);

Expand Down Expand Up @@ -535,7 +534,7 @@ namespace RTC
numItems,
this->buffer.size());

const auto intendedBufferSize = this->buffer.size() - numItems;
const size_t intendedBufferSize = this->buffer.size() - numItems;

while (this->buffer.size() > intendedBufferSize)
{
Expand Down
2 changes: 1 addition & 1 deletion worker/src/RTC/RtxStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ namespace RTC
if (this->lastSrReceived != 0)
{
// Get delay in milliseconds.
auto delayMs = static_cast<uint32_t>(DepLibUV::GetTimeMs() - this->lastSrReceived);
const uint32_t delayMs = DepLibUV::GetTimeMs() - this->lastSrReceived;
// Express delay in units of 1/65536 seconds.
uint32_t dlsr = (delayMs / 1000) << 16;

Expand Down
4 changes: 2 additions & 2 deletions worker/src/RTC/TransportCongestionControlServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,8 @@ namespace RTC
// the condition is met.
if (nowMs >= PacketArrivalTimestampWindow)
{
auto expiryTimestamp = nowMs - PacketArrivalTimestampWindow;
auto it = this->mapPacketArrivalTimes.begin();
uint64_t expiryTimestamp = nowMs - PacketArrivalTimestampWindow;
auto it = this->mapPacketArrivalTimes.begin();

while (it != this->mapPacketArrivalTimes.end() &&
it->first != this->transportCcFeedbackWideSeqNumStart &&
Expand Down
Loading