From dc7891c4b9b0213b83f21c34b013cece587171aa Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Mon, 2 Oct 2017 14:22:12 -0400
Subject: [PATCH] fix various issues with IpAddr interface
* prevent copy-initialisation with integer values
(maybe not what we want to do)
* remove by-pointer ctor API
(we don't need nullptr case)
* fix constness misuse on sockaddr conversion methods
* fix constness of bool conversion
(for cases where the IpAddr instance is not const)
* fix IpAddr rich-comparators (add <=, >=, <, >)
(fix usage of IpAddr as key in a map, where
the comparator is std::less, operator== was not used)
Change-Id: Iba9cf641d68bda37bd32d85dbd9dfffdc99078ed
Reviewed-by: Olivier Soldano <olivier.soldano@savoirfairelinux.com>
---
src/ip_utils.h | 59 +++++++++++++++++++++++++++------------
src/media/socket_pair.cpp | 6 ++--
2 files changed, 44 insertions(+), 21 deletions(-)
diff --git a/src/ip_utils.h b/src/ip_utils.h
index 77e8965ea0..0d4f41074d 100644
--- a/src/ip_utils.h
+++ b/src/ip_utils.h
@@ -67,37 +67,52 @@ namespace ring {
*/
class IpAddr {
public:
- IpAddr(uint16_t family = AF_UNSPEC) : addr() {
+ IpAddr() : IpAddr(AF_UNSPEC) {}
+ IpAddr(const IpAddr&) = default;
+ IpAddr(IpAddr&&) = default;
+ IpAddr& operator=(const IpAddr&) = default;
+ IpAddr& operator=(IpAddr&&) = default;
+
+ explicit IpAddr(uint16_t family) : addr() {
addr.addr.sa_family = family;
}
- // From a sockaddr-type structure
- IpAddr(const IpAddr& other) : addr(other.addr) {}
IpAddr(const pj_sockaddr& ip) : addr(ip) {}
- IpAddr(const sockaddr* ip, socklen_t len) : addr() {
- memcpy(&addr, ip, len);
+
+ IpAddr(const pj_sockaddr& ip, socklen_t len) : addr() {
+ if (len > sizeof(addr))
+ throw std::invalid_argument("IpAddr(): length overflows internal storage type");
+ memcpy(&addr, &ip, len);
+ }
+
+ IpAddr(const sockaddr& ip) : addr() {
+ memcpy(&addr, &ip, ip.sa_family == AF_INET6 ? sizeof addr.ipv6 : sizeof addr.ipv4);
}
IpAddr(const sockaddr_in& ip) : addr() {
+ static_assert(sizeof(ip) <= sizeof(addr), "sizeof(sockaddr_in) too large");
memcpy(&addr, &ip, sizeof(sockaddr_in));
}
+
IpAddr(const sockaddr_in6& ip) : addr() {
+ static_assert(sizeof(ip) <= sizeof(addr), "sizeof(sockaddr_in6) too large");
memcpy(&addr, &ip, sizeof(sockaddr_in6));
}
- IpAddr(const sockaddr& ip) : addr() {
- memcpy(&addr, &ip, ip.sa_family == AF_INET6 ? sizeof addr.ipv6 : sizeof addr.ipv4);
- }
+
IpAddr(const sockaddr_storage& ip) : IpAddr(*reinterpret_cast<const sockaddr*>(&ip)) {}
- IpAddr(const in6_addr& ip) : addr() {
- addr.addr.sa_family = AF_INET6;
- memcpy(&addr.ipv6.sin6_addr, &ip, sizeof(in6_addr));
- }
+
IpAddr(const in_addr& ip) : addr() {
+ static_assert(sizeof(ip) <= sizeof(addr), "sizeof(in_addr) too large");
addr.addr.sa_family = AF_INET;
memcpy(&addr.ipv4.sin_addr, &ip, sizeof(in_addr));
}
- // From a string
+ IpAddr(const in6_addr& ip) : addr() {
+ static_assert(sizeof(ip) <= sizeof(addr), "sizeof(in6_addr) too large");
+ addr.addr.sa_family = AF_INET6;
+ memcpy(&addr.ipv6.sin6_addr, &ip, sizeof(in6_addr));
+ }
+
IpAddr(const std::string& str, pj_uint16_t family = AF_UNSPEC) : addr() {
if (str.empty()) {
addr.addr.sa_family = AF_UNSPEC;
@@ -115,6 +130,10 @@ public:
return isIpv4() or isIpv6();
}
+ inline explicit operator bool() {
+ return isIpv4() or isIpv6();
+ }
+
inline operator pj_sockaddr& () {
return addr;
}
@@ -141,12 +160,12 @@ public:
return addr.ipv6;
}
- inline operator sockaddr& (){
- return reinterpret_cast<sockaddr&>(addr);
+ inline operator const sockaddr& () const {
+ return reinterpret_cast<const sockaddr&>(addr);
}
- inline explicit operator sockaddr* (){
- return reinterpret_cast<sockaddr*>(&addr);
+ inline operator const sockaddr* () const {
+ return reinterpret_cast<const sockaddr*>(&addr);
}
inline operator sockaddr_storage (){
@@ -229,12 +248,16 @@ public:
static bool isValid(const std::string& address, pj_uint16_t family = pj_AF_UNSPEC());
private:
- pj_sockaddr addr;
+ pj_sockaddr addr {};
};
// IpAddr helpers
inline bool operator==(const IpAddr& lhs, const IpAddr& rhs) { return !pj_sockaddr_cmp(&lhs, &rhs); }
inline bool operator!=(const IpAddr& lhs, const IpAddr& rhs) { return !(lhs == rhs); }
+inline bool operator<(const IpAddr& lhs, const IpAddr& rhs) { return pj_sockaddr_cmp(&lhs, &rhs) < 0; }
+inline bool operator>(const IpAddr& lhs, const IpAddr& rhs) { return pj_sockaddr_cmp(&lhs, &rhs) > 0; }
+inline bool operator<=(const IpAddr& lhs, const IpAddr& rhs) { return pj_sockaddr_cmp(&lhs, &rhs) <= 0; }
+inline bool operator>=(const IpAddr& lhs, const IpAddr& rhs) { return pj_sockaddr_cmp(&lhs, &rhs) >= 0; }
namespace ip_utils {
diff --git a/src/media/socket_pair.cpp b/src/media/socket_pair.cpp
index f4ce5c6fc1..ff32403803 100644
--- a/src/media/socket_pair.cpp
+++ b/src/media/socket_pair.cpp
@@ -150,7 +150,7 @@ udp_socket_create(int family, int port)
bind_addr.setPort(port);
RING_DBG("use local address: %s", bind_addr.toString(true, true).c_str());
- if (bind(udp_fd, (sockaddr*)bind_addr, bind_addr.getLength()) < 0) {
+ if (::bind(udp_fd, bind_addr, bind_addr.getLength()) < 0) {
RING_ERR("bind() failed");
strErr();
close(udp_fd);
@@ -455,8 +455,8 @@ SocketPair::writeData(uint8_t* buf, int buf_size)
if (noWrite_)
return buf_size;
- return sendto(fd, reinterpret_cast<const char*>(buf), buf_size, 0,
- (sockaddr*)*dest_addr, dest_addr->getLength());
+ return ::sendto(fd, reinterpret_cast<const char*>(buf), buf_size, 0,
+ *dest_addr, dest_addr->getLength());
}
if (noWrite_)
--
GitLab