Fix multi-field < ordering

Lots and lots of places in the code had broken < operators because they
are returning something like:

    foo < other.foo or bar < other.bar;

but this breaks both the strict weak ordering requirements that are
required for the "Compare" requirement for things like
std::map/set/priority_queue.

For example:

    a = {.foo=1, .bar=3}
    b = {.foo=3, .bar=1}

does not have an ordering over a and b (both `a < b` and `b < a` are
satisfied at the same time).

This needs to be instead something like:

    foo < other.foo or (foo == other.foo and bar < other.bar)

but that's a bit clunkier, and it is easier to use std::tie for tuple's
built-in < comparison which does the right thing:

    std::tie(foo, bar) < std::tie(other.foo, other.bar)

(Initially I noticed this in SockAddr/sockaddr_in6, but upon further
investigation this extends to the major of multi-field `operator<`'s.)

This fixes it by using std::tie (or something similar) everywhere we are
doing multi-field inequalities.
pull/2014/head
Jason Rhinelander 2 years ago
parent 54fba30516
commit 36792d4337
No known key found for this signature in database
GPG Key ID: C4992CE7A88D4262

@ -10,7 +10,7 @@
namespace llarp::dns namespace llarp::dns
{ {
typedef std::tuple<std::string, uint16_t, uint16_t, uint16_t, std::string> SRVTuple; using SRVTuple = std::tuple<std::string, uint16_t, uint16_t, uint16_t, std::string>;
struct SRVData struct SRVData
{ {
@ -38,19 +38,23 @@ namespace llarp::dns
SRVTuple SRVTuple
toTuple() const; toTuple() const;
auto
toTupleRef() const
{
return std::tie(service_proto, priority, weight, port, target);
}
/// so we can put SRVData in a std::set /// so we can put SRVData in a std::set
bool bool
operator<(const SRVData& other) const operator<(const SRVData& other) const
{ {
return service_proto < other.service_proto or priority < other.priority return toTupleRef() < other.toTupleRef();
or weight < other.weight or port < other.port or target < other.target;
} }
bool bool
operator==(const SRVData& other) const operator==(const SRVData& other) const
{ {
return service_proto == other.service_proto and priority == other.priority return toTupleRef() == other.toTupleRef();
and weight == other.weight and port == other.port and target == other.target;
} }
bool bool

@ -26,7 +26,7 @@ namespace llarp::iwp
, m_Inbound{allowInbound} , m_Inbound{allowInbound}
{} {}
const char* std::string_view
LinkLayer::Name() const LinkLayer::Name() const
{ {
return "iwp"; return "iwp";

@ -35,7 +35,7 @@ namespace llarp::iwp
std::shared_ptr<ILinkSession> std::shared_ptr<ILinkSession>
NewOutboundSession(const RouterContact& rc, const AddressInfo& ai) override; NewOutboundSession(const RouterContact& rc, const AddressInfo& ai) override;
const char* std::string_view
Name() const override; Name() const override;
uint16_t uint16_t

@ -53,11 +53,12 @@ namespace llarp
uint16_t m_ResendPriority; uint16_t m_ResendPriority;
bool bool
operator<(const OutboundMessage& msg) const operator<(const OutboundMessage& other) const
{ {
// yes, the first order is reversed as higher means more important // yes, the first order is reversed as higher means more important
// second part is for queue order // second part is for queue order
return msg.m_ResendPriority < m_ResendPriority or m_MsgID < msg.m_MsgID; int prioA = -m_ResendPriority, prioB = -other.m_ResendPriority;
return std::tie(prioA, m_MsgID) < std::tie(prioB, other.m_MsgID);
} }
ILinkSession::Packet_t ILinkSession::Packet_t

@ -249,7 +249,7 @@ namespace llarp
bool bool
ILinkLayer::PickAddress(const RouterContact& rc, llarp::AddressInfo& picked) const ILinkLayer::PickAddress(const RouterContact& rc, llarp::AddressInfo& picked) const
{ {
std::string OurDialect = Name(); auto OurDialect = Name();
for (const auto& addr : rc.addrs) for (const auto& addr : rc.addrs)
{ {
if (addr.dialect == OurDialect) if (addr.dialect == OurDialect)

@ -135,7 +135,7 @@ namespace llarp
virtual void virtual void
Stop(); Stop();
virtual const char* virtual std::string_view
Name() const = 0; Name() const = 0;
util::StatusObject util::StatusObject
@ -179,7 +179,7 @@ namespace llarp
bool bool
IsCompatable(const llarp::RouterContact& other) const IsCompatable(const llarp::RouterContact& other) const
{ {
const std::string us = Name(); const auto us = Name();
for (const auto& ai : other.addrs) for (const auto& ai : other.addrs)
if (ai.dialect == us) if (ai.dialect == us)
return true; return true;
@ -207,7 +207,9 @@ namespace llarp
bool bool
operator<(const ILinkLayer& other) const operator<(const ILinkLayer& other) const
{ {
return Rank() < other.Rank() || Name() < other.Name() || m_ourAddr < other.m_ourAddr; auto rankA = Rank(), rankB = other.Rank();
auto nameA = Name(), nameB = other.Name();
return std::tie(rankA, nameA, m_ourAddr) < std::tie(rankB, nameB, other.m_ourAddr);
} }
/// called by link session to remove a pending session who is timed out /// called by link session to remove a pending session who is timed out

@ -23,7 +23,7 @@ namespace llarp
bool bool
operator<(const AddressInfo& lhs, const AddressInfo& rhs) operator<(const AddressInfo& lhs, const AddressInfo& rhs)
{ {
return lhs.rank < rhs.rank || lhs.ip < rhs.ip || lhs.port < rhs.port; return std::tie(lhs.rank, lhs.ip, lhs.port) < std::tie(rhs.rank, rhs.ip, rhs.port);
} }
std::variant<nuint32_t, nuint128_t> std::variant<nuint32_t, nuint128_t>

@ -117,8 +117,8 @@ namespace llarp
bool bool
operator<(const IPRange& other) const operator<(const IPRange& other) const
{ {
return (this->addr & this->netmask_bits) < (other.addr & other.netmask_bits) auto maskedA = addr & netmask_bits, maskedB = other.addr & other.netmask_bits;
|| this->netmask_bits < other.netmask_bits; return std::tie(maskedA, netmask_bits) < std::tie(maskedB, other.netmask_bits);
} }
bool bool

@ -34,62 +34,16 @@
namespace llarp namespace llarp
{ {
inline bool inline int
operator==(const in_addr& a, const in_addr& b) cmp(const in_addr& a, const in_addr& b)
{ {
return memcmp(&a, &b, sizeof(in_addr)) == 0; return memcmp(&a, &b, sizeof(in_addr));
} }
inline bool inline int
operator==(const in6_addr& a, const in6_addr& b) cmp(const in6_addr& a, const in6_addr& b)
{ {
return memcmp(&a, &b, sizeof(in6_addr)) == 0; return memcmp(&a, &b, sizeof(in6_addr));
}
inline bool
operator==(const sockaddr_in& a, const sockaddr_in& b)
{
return a.sin_port == b.sin_port and a.sin_addr.s_addr == b.sin_addr.s_addr;
}
inline bool
operator==(const sockaddr_in6& a, const sockaddr_in6& b)
{
return a.sin6_port == b.sin6_port and a.sin6_addr == b.sin6_addr;
}
inline bool
operator==(const sockaddr& a, const sockaddr& b)
{
if (a.sa_family != b.sa_family)
return false;
switch (a.sa_family)
{
case AF_INET:
return reinterpret_cast<const sockaddr_in&>(a) == reinterpret_cast<const sockaddr_in&>(b);
case AF_INET6:
return reinterpret_cast<const sockaddr_in6&>(a) == reinterpret_cast<const sockaddr_in6&>(b);
default:
return false;
}
}
inline bool
operator<(const in_addr& a, const in_addr& b)
{
return memcmp(&a, &b, sizeof(in_addr)) < 0;
}
inline bool
operator<(const in6_addr& a, const in6_addr& b)
{
return memcmp(&a, &b, sizeof(in6_addr)) < 0;
}
inline bool
operator<(const sockaddr_in6& a, const sockaddr_in6& b)
{
return a.sin6_addr < b.sin6_addr or a.sin6_port < b.sin6_port;
} }
namespace net namespace net
@ -261,3 +215,61 @@ namespace llarp
} // namespace net } // namespace net
} // namespace llarp } // namespace llarp
inline bool
operator==(const in_addr& a, const in_addr& b)
{
return llarp::cmp(a, b) == 0;
}
inline bool
operator==(const in6_addr& a, const in6_addr& b)
{
return llarp::cmp(a, b) == 0;
}
inline bool
operator==(const sockaddr_in& a, const sockaddr_in& b)
{
return a.sin_port == b.sin_port and a.sin_addr.s_addr == b.sin_addr.s_addr;
}
inline bool
operator==(const sockaddr_in6& a, const sockaddr_in6& b)
{
return a.sin6_port == b.sin6_port and a.sin6_addr == b.sin6_addr;
}
inline bool
operator==(const sockaddr& a, const sockaddr& b)
{
if (a.sa_family != b.sa_family)
return false;
switch (a.sa_family)
{
case AF_INET:
return reinterpret_cast<const sockaddr_in&>(a) == reinterpret_cast<const sockaddr_in&>(b);
case AF_INET6:
return reinterpret_cast<const sockaddr_in6&>(a) == reinterpret_cast<const sockaddr_in6&>(b);
default:
return false;
}
}
inline bool
operator<(const in_addr& a, const in_addr& b)
{
return llarp::cmp(a, b) < 0;
}
inline bool
operator<(const in6_addr& a, const in6_addr& b)
{
return llarp::cmp(a, b) < 0;
}
inline bool
operator<(const sockaddr_in6& a, const sockaddr_in6& b)
{
return std::tie(a.sin6_addr, a.sin6_port) < std::tie(b.sin6_addr, b.sin6_port);
}

@ -206,15 +206,13 @@ namespace llarp
bool bool
SockAddr::operator<(const SockAddr& other) const SockAddr::operator<(const SockAddr& other) const
{ {
return (m_addr.sin6_addr < other.m_addr.sin6_addr) return m_addr < other.m_addr;
or (m_addr.sin6_port < other.m_addr.sin6_port);
} }
bool bool
SockAddr::operator==(const SockAddr& other) const SockAddr::operator==(const SockAddr& other) const
{ {
return m_addr.sin6_addr == other.m_addr.sin6_addr return m_addr == other.m_addr;
and m_addr.sin6_port == other.m_addr.sin6_port;
} }
huint128_t huint128_t

@ -33,11 +33,7 @@ namespace llarp::net
bool bool
operator<(const ProtocolInfo& other) const operator<(const ProtocolInfo& other) const
{ {
if (port and other.port) return std::tie(protocol, port) < std::tie(other.protocol, other.port);
{
return protocol < other.protocol or *port < *other.port;
}
return protocol < other.protocol;
} }
ProtocolInfo() = default; ProtocolInfo() = default;

@ -141,25 +141,25 @@ namespace llarp
constexpr bool constexpr bool
operator<(const uint128_t& b) const operator<(const uint128_t& b) const
{ {
return upper < b.upper || (upper == b.upper && lower < b.lower); return std::tie(upper, lower) < std::tie(b.upper, b.lower);
} }
constexpr bool constexpr bool
operator<=(const uint128_t& b) const operator<=(const uint128_t& b) const
{ {
return upper < b.upper || (upper == b.upper && lower <= b.lower); return std::tie(upper, lower) <= std::tie(b.upper, b.lower);
} }
constexpr bool constexpr bool
operator>(const uint128_t& b) const operator>(const uint128_t& b) const
{ {
return upper > b.upper || (upper == b.upper && lower > b.lower); return std::tie(upper, lower) > std::tie(b.upper, b.lower);
} }
constexpr bool constexpr bool
operator>=(const uint128_t& b) const operator>=(const uint128_t& b) const
{ {
return upper > b.upper || (upper == b.upper && lower >= b.lower); return std::tie(upper, lower) >= std::tie(b.upper, b.lower);
} }
constexpr uint128_t& constexpr uint128_t&

@ -41,7 +41,7 @@ namespace llarp
bool bool
operator<(const RouterVersion& other) const operator<(const RouterVersion& other) const
{ {
return m_ProtoVersion < other.m_ProtoVersion || m_Version < other.m_Version; return std::tie(m_ProtoVersion, m_Version) < std::tie(other.m_ProtoVersion, other.m_Version);
} }
bool bool

@ -55,8 +55,8 @@ namespace llarp
bool bool
operator<(const Introduction& other) const operator<(const Introduction& other) const
{ {
return expiresAt < other.expiresAt || pathID < other.pathID || router < other.router return std::tie(expiresAt, pathID, router, version, latency)
|| version < other.version || latency < other.latency; < std::tie(other.expiresAt, other.pathID, other.router, other.version, other.latency);
} }
bool bool

@ -25,7 +25,7 @@ namespace llarp::vpn
bool bool
operator<(const InterfaceAddress& other) const operator<(const InterfaceAddress& other) const
{ {
return range < other.range or fam < other.fam; return std::tie(range, fam) < std::tie(other.range, other.fam);
} }
}; };

Loading…
Cancel
Save