Change pointers to absl::optional where possible
This changelist adds a targeted pull of the absl::optional type,
replacing raw pointers where feasible, using the rules suggested
in the abseil documentation:
https://abseil.io/tips/123
There are remaining raw pointers, however I believe that these
should be changed to std::unique_ptr or simple references,
due to their scope and complexity. These cases include the following:
api/impl
* internal_services (Singleton)
api/impl/quic
* quic_connection
* quic_connection_factory
* quic_service_common
* fake_quic_connection_factory
* fake_quic_connection
api/public
* network_service_manager
* presentation_controller
* presentation_receiver
discovery/mdns
* mdns_responder*platform
tools/cddl
* codegen
* parse
* sema
Bug: openscreen:4
Change-Id: Id330a1b6529531b52e9d77f3a6f1b375f5a048d3
Reviewed-on: https://chromium-review.googlesource.com/c/1412262
Commit-Queue: Jordan Bayles <jophba@chromium.org>
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
diff --git a/.gitmodules b/.gitmodules
index 5df9ce2..984ed1f 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -15,3 +15,6 @@
[submodule "third_party/tinycbor/src"]
path = third_party/tinycbor/src
url = https://github.com/intel/tinycbor.git
+[submodule "third_party/abseil/src"]
+ path = third_party/abseil/src
+ url = https://github.com/abseil/abseil-cpp.git
diff --git a/BUILD.gn b/BUILD.gn
index 14c270a..ec221f7 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -11,6 +11,7 @@
"//discovery/mdns:embedder_demo",
"//platform",
"//sample:hello",
+ "//third_party/abseil",
"//third_party/chromium_quic",
"//third_party/chromium_quic:demo_client",
"//third_party/chromium_quic:demo_server",
@@ -52,6 +53,7 @@
"//discovery/mdns:mdns_unittests",
"//msgs:unittests",
"//sample:hello_unittests",
+ "//third_party/abseil",
"//third_party/googletest:gtest_main",
]
}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index c6ca314..d53a4df 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -40,6 +40,7 @@
"impl/quic:test_support",
"impl/testing",
"impl/testing:fakes_unittests",
+ "//third_party/abseil",
"//third_party/googletest:gmock",
"//third_party/googletest:gtest",
]
diff --git a/api/impl/quic/quic_connection_impl.cc b/api/impl/quic/quic_connection_impl.cc
index 884c9c2..1ca2e63 100644
--- a/api/impl/quic/quic_connection_impl.cc
+++ b/api/impl/quic/quic_connection_impl.cc
@@ -6,6 +6,7 @@
#include "api/impl/quic/quic_connection_factory_impl.h"
#include "base/make_unique.h"
+#include "third_party/abseil/src/absl/types/optional.h"
#include "third_party/chromium_quic/src/net/third_party/quic/platform/impl/quic_chromium_clock.h"
namespace openscreen {
@@ -21,7 +22,8 @@
int UdpTransport::Write(const char* buffer,
size_t buffer_length,
const PacketInfo& info) {
- return platform::SendUdp(socket_, buffer, buffer_length, destination_);
+ return platform::SendUdp(socket_, buffer, buffer_length, destination_)
+ .value_or(-1);
}
QuicStreamImpl::QuicStreamImpl(QuicStream::Delegate* delegate,
diff --git a/api/impl/testing/BUILD.gn b/api/impl/testing/BUILD.gn
index a6aaeea..db6084e 100644
--- a/api/impl/testing/BUILD.gn
+++ b/api/impl/testing/BUILD.gn
@@ -28,6 +28,7 @@
deps = [
":testing",
+ "//third_party/abseil",
"//third_party/googletest:gtest",
]
}
diff --git a/base/BUILD.gn b/base/BUILD.gn
index 55db4f5..90ed46f 100644
--- a/base/BUILD.gn
+++ b/base/BUILD.gn
@@ -17,6 +17,10 @@
"with_destruction_callback.cc",
"with_destruction_callback.h",
]
+
+ deps = [
+ "//third_party/abseil",
+ ]
}
source_set("base_unittests") {
diff --git a/base/ip_address.cc b/base/ip_address.cc
index 5fde169..4ae688c 100644
--- a/base/ip_address.cc
+++ b/base/ip_address.cc
@@ -8,6 +8,7 @@
#include <iomanip>
#include "platform/api/logging.h"
+#include "third_party/abseil/src/absl/types/optional.h"
namespace openscreen {
@@ -146,12 +147,12 @@
uint8_t values[16];
int i = 0;
int num_previous_colons = 0;
- int double_colon_index = -1;
+ absl::optional<int> double_colon_index = absl::nullopt;
for (auto c : s) {
if (c == ':') {
++num_previous_colons;
if (num_previous_colons == 2) {
- if (double_colon_index != -1) {
+ if (double_colon_index) {
return false;
}
double_colon_index = i;
@@ -189,12 +190,11 @@
values[i++] = static_cast<uint8_t>(next_value >> 8);
values[i] = static_cast<uint8_t>(next_value & 0xff);
- if (!((i == 15 && double_colon_index == -1) ||
- (i < 14 && double_colon_index != -1))) {
+ if (!((i == 15 && !double_colon_index) || (i < 14 && double_colon_index))) {
return false;
}
for (int j = 15; j >= 0;) {
- if (i == double_colon_index) {
+ if (double_colon_index && (i == double_colon_index)) {
address->bytes_[j--] = values[i--];
while (j > i)
address->bytes_[j--] = 0;
diff --git a/platform/BUILD.gn b/platform/BUILD.gn
index fbfc310..08a93d3 100644
--- a/platform/BUILD.gn
+++ b/platform/BUILD.gn
@@ -12,4 +12,8 @@
} else if (is_mac) {
deps += [ "mac" ]
}
+
+ public_deps = [
+ "//third_party/abseil",
+ ]
}
diff --git a/platform/api/BUILD.gn b/platform/api/BUILD.gn
index 65fb04d..8900278 100644
--- a/platform/api/BUILD.gn
+++ b/platform/api/BUILD.gn
@@ -17,5 +17,6 @@
public_deps = [
"//base",
+ "//third_party/abseil",
]
}
diff --git a/platform/api/socket.h b/platform/api/socket.h
index a97d1d6..caa57e9 100644
--- a/platform/api/socket.h
+++ b/platform/api/socket.h
@@ -7,6 +7,7 @@
#include "base/ip_address.h"
#include "platform/api/network_interface.h"
+#include "third_party/abseil/src/absl/types/optional.h"
namespace openscreen {
namespace platform {
@@ -35,15 +36,15 @@
const IPAddress& address,
InterfaceIndex ifindex);
-int64_t ReceiveUdp(UdpSocketPtr socket,
- void* data,
- int64_t length,
- IPEndpoint* src,
- IPEndpoint* original_destination);
-int64_t SendUdp(UdpSocketPtr socket,
- const void* data,
- int64_t length,
- const IPEndpoint& dest);
+absl::optional<int64_t> ReceiveUdp(UdpSocketPtr socket,
+ void* data,
+ int64_t length,
+ IPEndpoint* src,
+ IPEndpoint* original_destination);
+absl::optional<int64_t> SendUdp(UdpSocketPtr socket,
+ const void* data,
+ int64_t length,
+ const IPEndpoint& dest);
} // namespace platform
} // namespace openscreen
diff --git a/platform/base/event_loop.cc b/platform/base/event_loop.cc
index 2abd089..d4b7c84 100644
--- a/platform/base/event_loop.cc
+++ b/platform/base/event_loop.cc
@@ -17,18 +17,18 @@
bool ReceiveDataFromEvent(const UdpSocketReadableEvent& read_event,
ReceivedData* data) {
OSP_DCHECK(data);
- ssize_t len =
+ absl::optional<int> len =
ReceiveUdp(read_event.socket, &data->bytes[0], data->bytes.size(),
&data->source, &data->original_destination);
- if (len == -1) {
+ if (!len) {
OSP_LOG_ERROR << "recv() failed: " << GetLastErrorString();
return false;
} else if (len == 0) {
OSP_LOG_WARN << "recv() = 0, closed?";
return false;
}
- OSP_DCHECK_LE(len, kUdpMaxPacketSize);
- data->length = len;
+ OSP_DCHECK_LE(*len, kUdpMaxPacketSize);
+ data->length = *len;
data->socket = read_event.socket;
return true;
}
diff --git a/platform/linux/network_interface.cc b/platform/linux/network_interface.cc
index c4a8c05..81d4f08 100644
--- a/platform/linux/network_interface.cc
+++ b/platform/linux/network_interface.cc
@@ -99,7 +99,7 @@
info->type = GetInterfaceType(info->name);
}
-InterfaceAddresses* GetAddressesForIndex(
+InterfaceAddresses* FindOrAddAddressesForIndex(
std::vector<InterfaceAddresses>* address_list,
const std::vector<InterfaceInfo>& info_list,
InterfaceIndex index) {
@@ -360,7 +360,7 @@
struct ifaddrmsg* interface_address =
static_cast<struct ifaddrmsg*>(NLMSG_DATA(netlink_header));
- InterfaceAddresses* addresses = GetAddressesForIndex(
+ InterfaceAddresses* addresses = FindOrAddAddressesForIndex(
&address_list, info_list, interface_address->ifa_index);
if (!addresses) {
OSP_DVLOG(1) << "skipping address for interface "
diff --git a/platform/mac/network_interface.cc b/platform/mac/network_interface.cc
index 07c8251..c36cc8d 100644
--- a/platform/mac/network_interface.cc
+++ b/platform/mac/network_interface.cc
@@ -21,6 +21,7 @@
#include "base/ip_address.h"
#include "base/scoped_pipe.h"
+#include "platform/api/logging.h"
namespace openscreen {
namespace platform {
@@ -35,27 +36,32 @@
uint8_t ToPrefixLength(const uint8_t (&netmask)[N]) {
uint8_t result = 0;
size_t i = 0;
+
+ // Ensure all of the leftmost bits are set.
while (i < N && netmask[i] == UINT8_C(0xff)) {
result += 8;
++i;
}
- if (i < N) {
+
+ // Check the intermediate byte, the first that is not 0xFF,
+ // e.g. 0b11100000 or 0x00
+ if (i < N && netmask[i] != UINT8_C(0x00)) {
uint8_t last_byte = netmask[i];
+ // Check the left most bit, bitshifting as we go.
while (last_byte & UINT8_C(0x80)) {
++result;
last_byte <<= 1;
}
- if (last_byte != UINT8_C(0x00)) {
- return -1;
- }
+ OSP_CHECK(last_byte == UINT8_C(0x00));
++i;
}
+
+ // Ensure the rest of the bytes are zeroed out.
while (i < N) {
- if (netmask[i] != 0) {
- return -1;
- }
+ OSP_CHECK(netmask[i] == UINT8_C(0x00));
++i;
}
+
return result;
}
diff --git a/platform/posix/socket.cc b/platform/posix/socket.cc
index 7be9679..9243eae 100644
--- a/platform/posix/socket.cc
+++ b/platform/posix/socket.cc
@@ -17,6 +17,8 @@
#include "platform/api/logging.h"
#include "platform/posix/socket.h"
+#include "third_party/abseil/src/absl/types/optional.h"
+
namespace openscreen {
namespace platform {
namespace {
@@ -31,16 +33,16 @@
using IPv4InterfaceIndex = decltype(ip_mreqn().imr_ifindex);
using IPv6InterfaceIndex = decltype(ipv6_mreq().ipv6mr_interface);
-int CreateNonBlockingUdpSocket(int domain) {
+absl::optional<int> CreateNonBlockingUdpSocket(int domain) {
const int fd = socket(domain, SOCK_DGRAM, 0);
if (fd == -1) {
- return -1;
+ return absl::nullopt;
}
// On non-Linux, the SOCK_NONBLOCK option is not available, so use the
// more-portable method of calling fcntl() to set this behavior.
if (fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK) == -1) {
close(fd);
- return -1;
+ return absl::nullopt;
}
return fd;
}
@@ -48,15 +50,15 @@
} // namespace
UdpSocketPtr CreateUdpSocketIPv4() {
- const int fd = CreateNonBlockingUdpSocket(AF_INET);
- return fd == -1 ? nullptr
- : new UdpSocketPrivate{fd, UdpSocketPrivate::Version::kV4};
+ const absl::optional<int> fd = CreateNonBlockingUdpSocket(AF_INET);
+ return fd ? new UdpSocketPrivate{fd.value(), UdpSocketPrivate::Version::kV4}
+ : nullptr;
}
UdpSocketPtr CreateUdpSocketIPv6() {
- const int fd = CreateNonBlockingUdpSocket(AF_INET6);
- return fd == -1 ? nullptr
- : new UdpSocketPrivate{fd, UdpSocketPrivate::Version::kV6};
+ const absl::optional<int> fd = CreateNonBlockingUdpSocket(AF_INET6);
+ return fd ? new UdpSocketPrivate{fd.value(), UdpSocketPrivate::Version::kV6}
+ : nullptr;
}
bool IsIPv4Socket(UdpSocketPtr socket) {
@@ -183,11 +185,11 @@
}
}
-int64_t ReceiveUdp(UdpSocketPtr socket,
- void* data,
- int64_t length,
- IPEndpoint* src,
- IPEndpoint* original_destination) {
+absl::optional<int64_t> ReceiveUdp(UdpSocketPtr socket,
+ void* data,
+ int64_t length,
+ IPEndpoint* src,
+ IPEndpoint* original_destination) {
OSP_DCHECK_GE(socket->fd, 0);
struct iovec iov = {data, static_cast<size_t>(length)};
char control_buf[1024];
@@ -207,7 +209,7 @@
int64_t len = recvmsg(socket->fd, &msg, 0);
if (len < 0)
- return len;
+ return absl::nullopt;
src->address =
IPAddress(IPAddress::Version::kV4,
@@ -245,7 +247,7 @@
}
}
- return len;
+ return absl::optional<int>(len);
} else {
struct sockaddr_in6 sa;
struct msghdr msg;
@@ -302,44 +304,45 @@
}
}
-int64_t SendUdp(UdpSocketPtr socket,
- const void* data,
- int64_t length,
- const IPEndpoint& dest) {
+absl::optional<int64_t> SendUdp(UdpSocketPtr socket,
+ const void* data,
+ int64_t length,
+ const IPEndpoint& dest) {
OSP_DCHECK_GE(socket->fd, 0);
+
+ struct msghdr msg;
+ msg.msg_iovlen = 1;
+ msg.msg_control = nullptr;
+ msg.msg_controllen = 0;
+ msg.msg_flags = 0;
+
if (socket->version == UdpSocketPrivate::Version::kV4) {
struct sockaddr_in sa = {
.sin_family = AF_INET,
.sin_port = htons(dest.port),
};
- dest.address.CopyToV4(reinterpret_cast<uint8_t*>(&sa.sin_addr.s_addr));
- struct iovec iov = {const_cast<void*>(data), static_cast<size_t>(length)};
- struct msghdr msg;
msg.msg_name = &sa;
msg.msg_namelen = sizeof(sa);
+
+ dest.address.CopyToV4(reinterpret_cast<uint8_t*>(&sa.sin_addr.s_addr));
+ struct iovec iov = {const_cast<void*>(data), static_cast<size_t>(length)};
msg.msg_iov = &iov;
- msg.msg_iovlen = 1;
- msg.msg_control = nullptr;
- msg.msg_controllen = 0;
- msg.msg_flags = 0;
- return sendmsg(socket->fd, &msg, 0);
} else {
struct sockaddr_in6 sa = {
.sin6_family = AF_INET6,
.sin6_port = htons(dest.port),
};
- dest.address.CopyToV6(reinterpret_cast<uint8_t*>(&sa.sin6_addr.s6_addr));
- struct iovec iov = {const_cast<void*>(data), static_cast<size_t>(length)};
- struct msghdr msg;
msg.msg_name = &sa;
msg.msg_namelen = sizeof(sa);
+
+ dest.address.CopyToV6(reinterpret_cast<uint8_t*>(&sa.sin6_addr.s6_addr));
+ struct iovec iov = {const_cast<void*>(data), static_cast<size_t>(length)};
msg.msg_iov = &iov;
- msg.msg_iovlen = 1;
- msg.msg_control = nullptr;
- msg.msg_controllen = 0;
- msg.msg_flags = 0;
- return sendmsg(socket->fd, &msg, 0);
}
+
+ const int64_t return_value = sendmsg(socket->fd, &msg, 0);
+ return (return_value >= 0) ? absl::optional<int>(return_value)
+ : absl::optional<int>();
}
} // namespace platform
diff --git a/third_party/abseil/BUILD.gn b/third_party/abseil/BUILD.gn
new file mode 100644
index 0000000..d6e61b1
--- /dev/null
+++ b/third_party/abseil/BUILD.gn
@@ -0,0 +1,39 @@
+# Copyright 2018 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+config("abseil_config") {
+ include_dirs = [
+ "//third_party/abseil/src",
+ ]
+}
+
+source_set("abseil") {
+ sources = [
+ "src/absl/base/attributes.h",
+ "src/absl/base/config.h",
+ "src/absl/base/internal/atomic_hook.h",
+ "src/absl/base/internal/identity.h",
+ "src/absl/base/internal/inline_variable.h",
+ "src/absl/base/internal/invoke.h",
+ "src/absl/base/internal/raw_logging.cc",
+ "src/absl/base/internal/raw_logging.h",
+ "src/absl/base/log_severity.h",
+ "src/absl/base/macros.h",
+ "src/absl/base/optimization.h",
+ "src/absl/base/policy_checks.h",
+ "src/absl/base/port.h",
+ "src/absl/memory/memory.h",
+ "src/absl/meta/type_traits.h",
+ "src/absl/types/bad_optional_access.cc",
+ "src/absl/types/bad_optional_access.h",
+ "src/absl/types/optional.cc",
+ "src/absl/types/optional.h",
+ ]
+
+ configs -= [ "//build/config:symbol_visibility_hidden" ]
+ configs += [ "//build/config:symbol_visibility_default" ]
+ configs -= [ "//build:default_include_dirs" ]
+
+ public_configs = [":abseil_config"]
+}
diff --git a/third_party/abseil/src b/third_party/abseil/src
new file mode 160000
index 0000000..5eea0f7
--- /dev/null
+++ b/third_party/abseil/src
@@ -0,0 +1 @@
+Subproject commit 5eea0f713c14ac17788b83e496f11903f8e2bbb0
