Deprecate void* forms of StreamInterface::Read and ::Write
Updates the code to use the new interfaces
Bug: webrtc:14632
Change-Id: I33b2a25b5968de0251e3cbc84076afc013ecef6e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282680
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38601}
diff --git a/p2p/base/dtls_transport.cc b/p2p/base/dtls_transport.cc
index 904a0cb..7d9c0b5 100644
--- a/p2p/base/dtls_transport.cc
+++ b/p2p/base/dtls_transport.cc
@@ -16,6 +16,7 @@
#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
+#include "api/array_view.h"
#include "api/dtls_transport_interface.h"
#include "api/rtc_event_log/rtc_event_log.h"
#include "logging/rtc_event_log/events/rtc_event_dtls_transport_state.h"
@@ -445,7 +446,12 @@
return ice_transport_->SendPacket(data, size, options);
} else {
- return (dtls_->WriteAll(data, size, NULL, NULL) == rtc::SR_SUCCESS)
+ size_t written;
+ int error;
+ return (dtls_->WriteAll(
+ rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data),
+ size),
+ written, error) == rtc::SR_SUCCESS)
? static_cast<int>(size)
: -1;
}
@@ -691,16 +697,17 @@
}
}
if (sig & rtc::SE_READ) {
- char buf[kMaxDtlsPacketLen];
+ uint8_t buf[kMaxDtlsPacketLen];
size_t read;
int read_error;
rtc::StreamResult ret;
// The underlying DTLS stream may have received multiple DTLS records in
// one packet, so read all of them.
do {
- ret = dtls_->Read(buf, sizeof(buf), &read, &read_error);
+ ret = dtls_->Read(buf, read, read_error);
if (ret == rtc::SR_SUCCESS) {
- SignalReadPacket(this, buf, read, rtc::TimeMicros(), 0);
+ SignalReadPacket(this, reinterpret_cast<const char*>(buf), read,
+ rtc::TimeMicros(), 0);
} else if (ret == rtc::SR_EOS) {
// Remote peer shut down the association with no error.
RTC_LOG(LS_INFO) << ToString() << ": DTLS transport closed by remote";
diff --git a/rtc_base/openssl_stream_adapter.cc b/rtc_base/openssl_stream_adapter.cc
index 61bf674..5fa1a3c 100644
--- a/rtc_base/openssl_stream_adapter.cc
+++ b/rtc_base/openssl_stream_adapter.cc
@@ -28,6 +28,7 @@
#include <utility>
#include <vector>
+#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/safe_conversions.h"
@@ -215,7 +216,8 @@
BIO_clear_retry_flags(b);
size_t read;
int error;
- StreamResult result = stream->Read(out, outl, &read, &error);
+ StreamResult result = stream->Read(
+ rtc::MakeArrayView(reinterpret_cast<uint8_t*>(out), outl), read, error);
if (result == SR_SUCCESS) {
return checked_cast<int>(read);
} else if (result == SR_BLOCK) {
@@ -232,7 +234,9 @@
BIO_clear_retry_flags(b);
size_t written;
int error;
- StreamResult result = stream->Write(in, inl, &written, &error);
+ StreamResult result = stream->Write(
+ rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(in), inl), written,
+ error);
if (result == SR_SUCCESS) {
return checked_cast<int>(written);
} else if (result == SR_BLOCK) {
@@ -557,17 +561,30 @@
//
// StreamInterface Implementation
//
-
+// Backwards compatible Write() method using deprecated API.
+// Needed because deprecated API is still =0 in API definition.
StreamResult OpenSSLStreamAdapter::Write(const void* data,
size_t data_len,
size_t* written,
int* error) {
- RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Write(" << data_len << ")";
+ // TODO(bugs.webrtc.org/14632): Consider doing
+ // RTC_CHECK_NOTREACHED(); when downstream usage is eliminated.
+ size_t dummy_written;
+ int dummy_error;
+ return Write(
+ rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data), data_len),
+ written ? *written : dummy_written, error ? *error : dummy_error);
+}
+
+StreamResult OpenSSLStreamAdapter::Write(rtc::ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) {
+ RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Write(" << data.size() << ")";
switch (state_) {
case SSL_NONE:
// pass-through in clear text
- return stream_->Write(data, data_len, written, error);
+ return stream_->Write(data, written, error);
case SSL_WAIT:
case SSL_CONNECTING:
@@ -582,31 +599,26 @@
case SSL_ERROR:
case SSL_CLOSED:
default:
- if (error) {
- *error = ssl_error_code_;
- }
+ error = ssl_error_code_;
return SR_ERROR;
}
// OpenSSL will return an error if we try to write zero bytes
- if (data_len == 0) {
- if (written) {
- *written = 0;
- }
+ if (data.size() == 0) {
+ written = 0;
return SR_SUCCESS;
}
ssl_write_needs_read_ = false;
- int code = SSL_write(ssl_, data, checked_cast<int>(data_len));
+ int code = SSL_write(ssl_, data.data(), checked_cast<int>(data.size()));
int ssl_error = SSL_get_error(ssl_, code);
switch (ssl_error) {
case SSL_ERROR_NONE:
RTC_DLOG(LS_VERBOSE) << " -- success";
RTC_DCHECK_GT(code, 0);
- RTC_DCHECK_LE(code, data_len);
- if (written)
- *written = code;
+ RTC_DCHECK_LE(code, data.size());
+ written = code;
return SR_SUCCESS;
case SSL_ERROR_WANT_READ:
RTC_DLOG(LS_VERBOSE) << " -- error want read";
@@ -619,23 +631,33 @@
case SSL_ERROR_ZERO_RETURN:
default:
Error("SSL_write", (ssl_error ? ssl_error : -1), 0, false);
- if (error) {
- *error = ssl_error_code_;
- }
+ error = ssl_error_code_;
return SR_ERROR;
}
// not reached
}
+// Backwards compatible Read() method using deprecated API.
StreamResult OpenSSLStreamAdapter::Read(void* data,
size_t data_len,
size_t* read,
int* error) {
- RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Read(" << data_len << ")";
+ // TODO(bugs.webrtc.org/14632): Consider doing
+ // RTC_CHECK_NOTREACHED() when downstream usage is thought to be eliminated.
+ size_t dummy_read;
+ int dummy_error;
+ return Read(rtc::MakeArrayView(reinterpret_cast<uint8_t*>(data), data_len),
+ read ? *read : dummy_read, error ? *error : dummy_error);
+}
+
+StreamResult OpenSSLStreamAdapter::Read(rtc::ArrayView<uint8_t> data,
+ size_t& read,
+ int& error) {
+ RTC_DLOG(LS_VERBOSE) << "OpenSSLStreamAdapter::Read(" << data.size() << ")";
switch (state_) {
case SSL_NONE:
// pass-through in clear text
- return stream_->Read(data, data_len, read, error);
+ return stream_->Read(data, read, error);
case SSL_WAIT:
case SSL_CONNECTING:
return SR_BLOCK;
@@ -648,33 +670,27 @@
return SR_EOS;
case SSL_ERROR:
default:
- if (error) {
- *error = ssl_error_code_;
- }
+ error = ssl_error_code_;
return SR_ERROR;
}
// Don't trust OpenSSL with zero byte reads
- if (data_len == 0) {
- if (read) {
- *read = 0;
- }
+ if (data.size() == 0) {
+ read = 0;
return SR_SUCCESS;
}
ssl_read_needs_write_ = false;
- const int code = SSL_read(ssl_, data, checked_cast<int>(data_len));
+ const int code = SSL_read(ssl_, data.data(), checked_cast<int>(data.size()));
const int ssl_error = SSL_get_error(ssl_, code);
switch (ssl_error) {
case SSL_ERROR_NONE:
RTC_DLOG(LS_VERBOSE) << " -- success";
RTC_DCHECK_GT(code, 0);
- RTC_DCHECK_LE(code, data_len);
- if (read) {
- *read = code;
- }
+ RTC_DCHECK_LE(code, data.size());
+ read = code;
if (ssl_mode_ == SSL_MODE_DTLS) {
// Enforce atomic reads -- this is a short read
@@ -683,9 +699,7 @@
if (pending) {
RTC_DLOG(LS_INFO) << " -- short DTLS read. flushing";
FlushInput(pending);
- if (error) {
- *error = SSE_MSG_TRUNC;
- }
+ error = SSE_MSG_TRUNC;
return SR_ERROR;
}
}
@@ -703,9 +717,7 @@
return SR_EOS;
default:
Error("SSL_read", (ssl_error ? ssl_error : -1), 0, false);
- if (error) {
- *error = ssl_error_code_;
- }
+ error = ssl_error_code_;
return SR_ERROR;
}
// not reached
diff --git a/rtc_base/openssl_stream_adapter.h b/rtc_base/openssl_stream_adapter.h
index 891f0e6..3c94ecd 100644
--- a/rtc_base/openssl_stream_adapter.h
+++ b/rtc_base/openssl_stream_adapter.h
@@ -95,14 +95,19 @@
void SetMaxProtocolVersion(SSLProtocolVersion version) override;
void SetInitialRetransmissionTimeout(int timeout_ms) override;
- StreamResult Read(void* data,
- size_t data_len,
- size_t* read,
- int* error) override;
- StreamResult Write(const void* data,
- size_t data_len,
- size_t* written,
- int* error) override;
+ [[deprecated("bugs.webrtc.org/14632")]] StreamResult
+ Read(void* data, size_t data_len, size_t* read, int* error) override;
+ StreamResult Read(rtc::ArrayView<uint8_t> data,
+ size_t& read,
+ int& error) override;
+ [[deprecated("bugs.webrtc.org/14632")]] StreamResult Write(
+ const void* data,
+ size_t data_len,
+ size_t* written,
+ int* error) override;
+ StreamResult Write(rtc::ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) override;
void Close() override;
StreamState GetState() const override;
diff --git a/rtc_base/ssl_adapter_unittest.cc b/rtc_base/ssl_adapter_unittest.cc
index cd63249..2da59dd 100644
--- a/rtc_base/ssl_adapter_unittest.cc
+++ b/rtc_base/ssl_adapter_unittest.cc
@@ -204,7 +204,9 @@
int error;
rtc::StreamResult r = ssl_stream_adapter_->Write(
- message.data(), message.length(), &written, &error);
+ rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(message.data()),
+ message.size()),
+ written, error);
if (r == rtc::SR_SUCCESS) {
return written;
} else {
@@ -236,18 +238,19 @@
void OnSSLStreamAdapterEvent(rtc::StreamInterface* stream, int sig, int err) {
if (sig & rtc::SE_READ) {
- char buffer[4096] = "";
+ uint8_t buffer[4096] = "";
size_t read;
int error;
// Read data received from the client and store it in our internal
// buffer.
- rtc::StreamResult r =
- stream->Read(buffer, sizeof(buffer) - 1, &read, &error);
+ rtc::StreamResult r = stream->Read(buffer, read, error);
if (r == rtc::SR_SUCCESS) {
buffer[read] = '\0';
- RTC_LOG(LS_INFO) << "Server received '" << buffer << "'";
- data_ += buffer;
+ // Here we assume that the buffer is interpretable as string.
+ char* buffer_as_char = reinterpret_cast<char*>(buffer);
+ RTC_LOG(LS_INFO) << "Server received '" << buffer_as_char << "'";
+ data_ += buffer_as_char;
}
}
}
diff --git a/rtc_base/ssl_stream_adapter_unittest.cc b/rtc_base/ssl_stream_adapter_unittest.cc
index f6a7e32..f8a015d 100644
--- a/rtc_base/ssl_stream_adapter_unittest.cc
+++ b/rtc_base/ssl_stream_adapter_unittest.cc
@@ -17,6 +17,7 @@
#include "absl/memory/memory.h"
#include "absl/strings/string_view.h"
+#include "api/array_view.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "rtc_base/buffer_queue.h"
#include "rtc_base/checks.h"
@@ -165,7 +166,9 @@
int* error) override {
rtc::StreamResult r;
- r = in_->Read(buffer, buffer_len, read, error);
+ r = in_->Read(
+ rtc::MakeArrayView(reinterpret_cast<uint8_t*>(buffer), buffer_len),
+ *read, *error);
if (r == rtc::SR_BLOCK)
return rtc::SR_BLOCK;
if (r == rtc::SR_EOS)
@@ -201,17 +204,15 @@
}
// Write to the outgoing FifoBuffer
- rtc::StreamResult WriteData(const void* data,
- size_t data_len,
- size_t* written,
- int* error) {
- return out_->Write(data, data_len, written, error);
+ rtc::StreamResult WriteData(rtc::ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) {
+ return out_->Write(data, written, error);
}
- rtc::StreamResult Write(const void* data,
- size_t data_len,
- size_t* written,
- int* error) override;
+ rtc::StreamResult Write(rtc::ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) override;
void Close() override {
RTC_LOG(LS_INFO) << "Closing outbound stream";
@@ -649,17 +650,17 @@
rtc::StreamResult DataWritten(SSLDummyStreamBase* from,
const void* data,
size_t data_len,
- size_t* written,
- int* error) {
+ size_t& written,
+ int& error) {
// Randomly drop loss_ percent of packets
if (rtc::CreateRandomId() % 100 < static_cast<uint32_t>(loss_)) {
RTC_LOG(LS_VERBOSE) << "Randomly dropping packet, size=" << data_len;
- *written = data_len;
+ written = data_len;
return rtc::SR_SUCCESS;
}
if (dtls_ && (data_len > mtu_)) {
RTC_LOG(LS_VERBOSE) << "Dropping packet > mtu, size=" << data_len;
- *written = data_len;
+ written = data_len;
return rtc::SR_SUCCESS;
}
@@ -667,17 +668,19 @@
// handshake packets and we damage the last byte to keep the header
// intact but break the MAC.
if (damage_ && (*static_cast<const unsigned char*>(data) == 23)) {
- std::vector<char> buf(data_len);
+ std::vector<uint8_t> buf(data_len);
RTC_LOG(LS_VERBOSE) << "Damaging packet";
memcpy(&buf[0], data, data_len);
buf[data_len - 1]++;
-
- return from->WriteData(&buf[0], data_len, written, error);
+ return from->WriteData(rtc::MakeArrayView(&buf[0], data_len), written,
+ error);
}
- return from->WriteData(data, data_len, written, error);
+ return from->WriteData(
+ rtc::MakeArrayView(reinterpret_cast<const uint8_t*>(data), data_len),
+ written, error);
}
void SetDelay(int delay) { delay_ = delay; }
@@ -838,7 +841,7 @@
size_t position, tosend, size;
rtc::StreamResult rv;
size_t sent;
- char block[kBlockSize];
+ uint8_t block[kBlockSize];
send_stream_.GetSize(&size);
if (!size)
@@ -848,7 +851,8 @@
send_stream_.GetPosition(&position);
if (send_stream_.Read(block, sizeof(block), &tosend, nullptr) !=
rtc::SR_EOS) {
- rv = client_ssl_->Write(block, tosend, &sent, 0);
+ int error;
+ rv = client_ssl_->Write(rtc::MakeArrayView(block, tosend), sent, error);
if (rv == rtc::SR_SUCCESS) {
send_stream_.SetPosition(position + sent);
@@ -871,13 +875,13 @@
}
void ReadData(rtc::StreamInterface* stream) override {
- char buffer[1600];
+ uint8_t buffer[1600];
size_t bread;
int err2;
rtc::StreamResult r;
for (;;) {
- r = stream->Read(buffer, sizeof(buffer), &bread, &err2);
+ r = stream->Read(buffer, bread, err2);
if (r == rtc::SR_ERROR || r == rtc::SR_EOS) {
// Unfortunately, errors are the way that the stream adapter
@@ -930,7 +934,7 @@
}
void WriteData() override {
- unsigned char* packet = new unsigned char[1600];
+ uint8_t* packet = new uint8_t[1600];
while (sent_ < count_) {
unsigned int rand_state = sent_;
@@ -942,7 +946,9 @@
}
size_t sent;
- rtc::StreamResult rv = client_ssl_->Write(packet, packet_size_, &sent, 0);
+ int error;
+ rtc::StreamResult rv = client_ssl_->Write(
+ rtc::MakeArrayView(packet, packet_size_), sent, error);
if (rv == rtc::SR_SUCCESS) {
RTC_LOG(LS_VERBOSE) << "Sent: " << sent_;
sent_++;
@@ -959,13 +965,13 @@
}
void ReadData(rtc::StreamInterface* stream) override {
- unsigned char buffer[2000];
+ uint8_t buffer[2000];
size_t bread;
int err2;
rtc::StreamResult r;
for (;;) {
- r = stream->Read(buffer, 2000, &bread, &err2);
+ r = stream->Read(buffer, bread, err2);
if (r == rtc::SR_ERROR) {
// Unfortunately, errors are the way that the stream adapter
@@ -1037,22 +1043,22 @@
: SSLStreamAdapterTestDTLSBase(cert_pem, private_key_pem) {}
};
-rtc::StreamResult SSLDummyStreamBase::Write(const void* data,
- size_t data_len,
- size_t* written,
- int* error) {
- RTC_LOG(LS_VERBOSE) << "Writing to loopback " << data_len;
+rtc::StreamResult SSLDummyStreamBase::Write(rtc::ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) {
+ RTC_LOG(LS_VERBOSE) << "Writing to loopback " << data.size();
if (first_packet_) {
first_packet_ = false;
if (test_base_->GetLoseFirstPacket()) {
- RTC_LOG(LS_INFO) << "Losing initial packet of length " << data_len;
- *written = data_len; // Fake successful writing also to writer.
+ RTC_LOG(LS_INFO) << "Losing initial packet of length " << data.size();
+ written = data.size(); // Fake successful writing also to writer.
return rtc::SR_SUCCESS;
}
}
- return test_base_->DataWritten(this, data, data_len, written, error);
+ return test_base_->DataWritten(this, data.data(), data.size(), written,
+ error);
}
class SSLStreamAdapterTestDTLSFromPEMStrings : public SSLStreamAdapterTestDTLS {
@@ -1169,15 +1175,16 @@
client_ssl_->Close();
rtc::StreamResult rv;
- char block[kBlockSize];
+ uint8_t block[kBlockSize];
size_t dummy;
+ int error;
// It's an error to write after closed.
- rv = client_ssl_->Write(block, sizeof(block), &dummy, nullptr);
+ rv = client_ssl_->Write(block, dummy, error);
ASSERT_EQ(rtc::SR_ERROR, rv);
// But after closed read gives you EOS.
- rv = client_ssl_->Read(block, sizeof(block), &dummy, nullptr);
+ rv = client_ssl_->Read(block, dummy, error);
ASSERT_EQ(rtc::SR_EOS, rv);
}
diff --git a/rtc_base/stream.cc b/rtc_base/stream.cc
index e1aab8c..e6b74b4 100644
--- a/rtc_base/stream.cc
+++ b/rtc_base/stream.cc
@@ -15,6 +15,7 @@
#include <algorithm>
#include <string>
+#include "api/array_view.h"
#include "rtc_base/checks.h"
#include "rtc_base/thread.h"
@@ -31,8 +32,10 @@
StreamResult result = SR_SUCCESS;
size_t total_written = 0, current_written;
while (total_written < data_len) {
- result = Write(static_cast<const char*>(data) + total_written,
- data_len - total_written, ¤t_written, error);
+ result = Write(ArrayView<const uint8_t>(
+ reinterpret_cast<const uint8_t*>(data) + total_written,
+ data_len - total_written),
+ current_written, *error);
if (result != SR_SUCCESS)
break;
total_written += current_written;
diff --git a/rtc_base/stream.h b/rtc_base/stream.h
index c57ee61..fa928fc 100644
--- a/rtc_base/stream.h
+++ b/rtc_base/stream.h
@@ -69,24 +69,48 @@
// block, or the stream is in SS_OPENING state.
// SR_EOS: the end-of-stream has been reached, or the stream is in the
// SS_CLOSED state.
- virtual StreamResult Read(void* buffer,
- size_t buffer_len,
- size_t* read,
- int* error) = 0;
+
+ // The deprecated method has a default implementation that may be
+ // overridden in subclasses, rather than being =0.
+ // This allows subclasses to delete the method.
+ // TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
+ [[deprecated("Use ArrayView version")]] virtual StreamResult
+ Read(void* buffer, size_t buffer_len, size_t* read, int* error) {
+ RTC_CHECK_NOTREACHED();
+ }
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ // Preserve backwards compatibility using a default implementation
+ // because there are subclasses
+ // outside of the WebRTC codebase that need to be converted.
+ //
+ // TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
virtual StreamResult Read(rtc::ArrayView<uint8_t> buffer,
size_t& read,
int& error) {
return Read(buffer.data(), buffer.size(), &read, &error);
}
- virtual StreamResult Write(const void* data,
- size_t data_len,
- size_t* written,
- int* error) = 0;
+#pragma clang diagnostic pop
+
+ // The deprecated method has a default implementation that may be
+ // overridden in subclasses, rather than being =0.
+ // This allows subclasses to delete the method.
+ // TODO(bugs.webrtc.org/14632): Remove when downstream is converted.
+ [[deprecated("Use ArrayView version")]] virtual StreamResult
+ Write(const void* data, size_t data_len, size_t* written, int* error) {
+ RTC_CHECK_NOTREACHED();
+ }
+
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
virtual StreamResult Write(rtc::ArrayView<const uint8_t> data,
size_t& written,
int& error) {
return Write(data.data(), data.size(), &written, &error);
}
+#pragma clang diagnostic pop
+
// Attempt to transition to the SS_CLOSED state. SE_CLOSE will not be
// signalled as a result of this call.
virtual void Close() = 0;
@@ -115,14 +139,19 @@
// unlike Write, the argument 'written' is always set, and may be non-zero
// on results other than SR_SUCCESS. The remaining arguments have the
// same semantics as Write.
- StreamResult WriteAll(const void* data,
- size_t data_len,
- size_t* written,
- int* error);
+ [[deprecated("Use version with ArrayView")]] StreamResult
+ WriteAll(const void* data, size_t data_len, size_t* written, int* error);
- StreamResult WriteAll(ArrayView<uint8_t> data, size_t& written, int& error) {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+ // TODO(bugs.webrc.org/14632): Remove pragmas and change underlying
+ // implementation when downstream code is converted.
+ StreamResult WriteAll(ArrayView<const uint8_t> data,
+ size_t& written,
+ int& error) {
return WriteAll(data.data(), data.size(), &written, &error);
}
+#pragma clang diagnostic pop
protected:
StreamInterface();
