libstdc++: Fix internet socket option classes
authorJonathan Wakely <jwakely@redhat.com>
Mon, 26 Apr 2021 20:16:21 +0000 (21:16 +0100)
committerJonathan Wakely <jwakely@redhat.com>
Mon, 26 Apr 2021 20:16:21 +0000 (21:16 +0100)
Similar to the previous commit, this fixes various problems with the
socket options classes in the <internet> header:

- The constructors were not noexcept.
- The __sockopt_base<T>::value() member function was present
  unconditionally (so was defined for socket_base::linger which is
  incorrect).
- The __socket_crtp<C, T>::operator=(T) assignment operator was not
  noexcept, and was hidden in the derived classes.
- The MulticastSocketOptions incorrectly used a union, incorrectly
  defined resize and const data() member functions, and their
  constructors were unimplemented.

Also, where appropriate:

- Use class instead of struct for the socket option types.
- Define the _S_level and _S_name constants as private.
- Declare the __socket_crtp base as a friend.

libstdc++-v3/ChangeLog:

* include/experimental/internet (tcp::no_delay, v6_only)
(unicast::hops, multicast::hops, multicast::enable_loopback):
Change access of base class and static data members. Add
using-declaration for __socket_crtp::operator=(_Tp).
(multicast::__mcastopt): New type.
(multicast::join_group, multicast::leave_group): Derive from
__mcastopt for common implementation.
* include/experimental/socket: Add comment.
* testsuite/experimental/net/internet/socket/opt.cc: New test.
* testsuite/experimental/net/socket/socket_base.cc: Check for
protected constructor/destructor of socket_base. Check for
explicit constructors of socket option classes.

libstdc++-v3/include/experimental/internet
libstdc++-v3/include/experimental/socket
libstdc++-v3/testsuite/experimental/net/internet/socket/opt.cc [new file with mode: 0644]
libstdc++-v3/testsuite/experimental/net/socket/socket_base.cc

index d3321af..2e3fd06 100644 (file)
@@ -52,7 +52,7 @@
 # include <arpa/inet.h>                // inet_ntop
 #endif
 #ifdef _GLIBCXX_HAVE_NETINET_IN_H
-# include <netinet/in.h>       // IPPROTO_IP
+# include <netinet/in.h>       // IPPROTO_IP, IPPROTO_IPV6, in_addr, in6_addr
 #endif
 #ifdef _GLIBCXX_HAVE_NETINET_TCP_H
 # include <netinet/tcp.h>      // TCP_NODELAY
@@ -2078,6 +2078,7 @@ namespace ip
     struct no_delay : __sockopt_crtp<no_delay, bool>
     {
       using __sockopt_crtp::__sockopt_crtp;
+      using __sockopt_crtp::operator=;
 
       static const int _S_level = IPPROTO_TCP;
       static const int _S_name = TCP_NODELAY;
@@ -2157,10 +2158,14 @@ namespace ip
   /// @}
 
   /// Restrict a socket created for an IPv6 protocol to IPv6 only.
-  struct v6_only : __sockopt_crtp<v6_only, bool>
+  class v6_only : public __sockopt_crtp<v6_only, bool>
   {
+  public:
     using __sockopt_crtp::__sockopt_crtp;
+    using __sockopt_crtp::operator=;
 
+  private:
+    friend __sockopt_crtp<v6_only, bool>;
     static const int _S_level = IPPROTO_IPV6;
     static const int _S_name = IPV6_V6ONLY;
   };
@@ -2168,9 +2173,11 @@ namespace ip
   namespace unicast
   {
     /// Set the default number of hops (TTL) for outbound datagrams.
-    struct hops : __sockopt_crtp<hops>
+    class hops : public __sockopt_crtp<hops>
     {
+    public:
       using __sockopt_crtp::__sockopt_crtp;
+      using __sockopt_crtp::operator=;
 
       template<typename _Protocol>
        int
@@ -2186,17 +2193,34 @@ namespace ip
 
   namespace multicast
   {
-    /// Request that a socket joins a multicast group.
-    struct join_group
+    class __mcastopt
     {
+    public:
       explicit
-      join_group(const address&);
+      __mcastopt(const address& __grp) noexcept
+      : __mcastopt(__grp.is_v4() ? __mcastopt(__grp.to_v4()) : __mcastopt(__grp.to_v6()))
+      { }
 
       explicit
-      join_group(const address_v4&, const address_v4& = address_v4::any());
+      __mcastopt(const address_v4& __grp,
+                const address_v4& __iface = address_v4::any()) noexcept
+      {
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+       _M_v4.imr_multiaddr.s_addr = __grp.to_uint();
+       _M_v4.imr_interface.s_addr = __iface.to_uint();
+#else
+       _M_v4.imr_multiaddr.s_addr = __builtin_bswap32(__grp.to_uint());
+       _M_v4.imr_interface.s_addr = __builtin_bswap32(__iface.to_uint());
+#endif
+      }
 
       explicit
-      join_group(const address_v6&, unsigned int = 0);
+      __mcastopt(const address_v6& __grp, unsigned int __iface = 0) noexcept
+      {
+       const auto __addr = __grp.to_bytes();
+       __builtin_memcpy(_M_v6.ipv6mr_multiaddr.s6_addr, __addr.data(), 16);
+       _M_v6.ipv6mr_interface = __iface;
+      }
 
       template<typename _Protocol>
        int
@@ -2204,112 +2228,76 @@ namespace ip
        { return __p.family() == AF_INET6 ? IPPROTO_IPV6 : IPPROTO_IP; }
 
       template<typename _Protocol>
-       int
-       name(const _Protocol& __p) const noexcept
-       {
-         return __p.family() == AF_INET6
-           ? IPV6_JOIN_GROUP : IP_ADD_MEMBERSHIP;
-       }
-      template<typename _Protocol>
-       void*
-       data(const _Protocol&) noexcept
-       { return std::addressof(_M_value); }
-
-      template<typename _Protocol>
        const void*
-       data(const _Protocol&) const noexcept
-       { return std::addressof(_M_value); }
+       data(const _Protocol& __p) const noexcept
+       { return __p.family() == AF_INET6 ? &_M_v6 : &_M_v4; }
 
       template<typename _Protocol>
        size_t
        size(const _Protocol& __p) const noexcept
-       {
-         return __p.family() == AF_INET6
-           ? sizeof(_M_value._M_v6) : sizeof(_M_value._M_v4);
-       }
-
-      template<typename _Protocol>
-       void
-       resize(const _Protocol& __p, size_t __s)
-       {
-         if (__s != size(__p))
-           __throw_length_error("invalid value for socket option resize");
-       }
+       { return __p.family() == AF_INET6 ? sizeof(_M_v6) : sizeof(_M_v4); }
 
-    protected:
-      union
-      {
-       ipv6_mreq _M_v6;
-       ip_mreq _M_v4;
-      } _M_value;
+    private:
+      ipv6_mreq _M_v6 = {};
+      ip_mreq _M_v4 = {};
     };
 
-    /// Request that a socket leaves a multicast group.
-    struct leave_group
+    /// Request that a socket joins a multicast group.
+    class join_group : private __mcastopt
     {
-      explicit
-      leave_group(const address&);
-
-      explicit
-      leave_group(const address_v4&, const address_v4& = address_v4::any());
-
-      explicit
-      leave_group(const address_v6&, unsigned int = 0);
-
-      template<typename _Protocol>
-       int
-       level(const _Protocol& __p) const noexcept
-       { return __p.family() == AF_INET6 ? IPPROTO_IPV6 : IPPROTO_IP; }
+    public:
+      using __mcastopt::__mcastopt;
+      using __mcastopt::level;
+      using __mcastopt::data;
+      using __mcastopt::size;
 
       template<typename _Protocol>
        int
        name(const _Protocol& __p) const noexcept
        {
-         return __p.family() == AF_INET6
-           ? IPV6_LEAVE_GROUP : IP_DROP_MEMBERSHIP;
+         if (__p.family() == AF_INET6)
+           return IPV6_JOIN_GROUP;
+         return IP_ADD_MEMBERSHIP;
        }
-      template<typename _Protocol>
-       void*
-       data(const _Protocol&) noexcept
-       { return std::addressof(_M_value); }
-
-      template<typename _Protocol>
-       const void*
-       data(const _Protocol&) const noexcept
-       { return std::addressof(_M_value); }
+    };
 
-      template<typename _Protocol>
-       size_t
-       size(const _Protocol& __p) const noexcept
-       {
-         return __p.family() == AF_INET6
-           ? sizeof(_M_value._M_v6) : sizeof(_M_value._M_v4);
-       }
+    /// Request that a socket leaves a multicast group.
+    class leave_group : private __mcastopt
+    {
+    public:
+      using __mcastopt::__mcastopt;
+      using __mcastopt::level;
+      using __mcastopt::data;
+      using __mcastopt::size;
 
       template<typename _Protocol>
-       void
-       resize(const _Protocol& __p, size_t __s)
+       int
+       name(const _Protocol& __p) const noexcept
        {
-         if (__s != size(__p))
-           __throw_length_error("invalid value for socket option resize");
+         if (__p.family() == AF_INET6)
+           return IPV6_LEAVE_GROUP;
+         return IP_DROP_MEMBERSHIP;
        }
-
-    protected:
-      union
-      {
-       ipv6_mreq _M_v6;
-       ip_mreq _M_v4;
-      } _M_value;
     };
 
     /// Specify the network interface for outgoing multicast datagrams.
     class outbound_interface
     {
+    public:
       explicit
-      outbound_interface(const address_v4&);
+      outbound_interface(const address_v4& __v4) noexcept
+      {
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+       _M_v4.s_addr = __v4.to_uint();
+#else
+       _M_v4.s_addr = __builtin_bswap32(__v4.to_uint());
+#endif
+      }
 
       explicit
-      outbound_interface(unsigned int);
+      outbound_interface(unsigned int __v6) noexcept
+      : _M_v4(), _M_v6(__v6)
+      { }
 
       template<typename _Protocol>
        int
@@ -2326,28 +2314,25 @@ namespace ip
 
       template<typename _Protocol>
        const void*
-       data(const _Protocol&) const noexcept
-       { return std::addressof(_M_value); }
+       data(const _Protocol& __p) const noexcept
+       { return __p.family() == AF_INET6 ? &_M_v6 : &_M_v4; }
 
       template<typename _Protocol>
        size_t
        size(const _Protocol& __p) const noexcept
-       {
-         return __p.family() == AF_INET6
-           ? sizeof(_M_value._M_v6) : sizeof(_M_value._M_v4);
-       }
+       { return __p.family() == AF_INET6 ? sizeof(_M_v6) : sizeof(_M_v4); }
 
-    protected:
-      union {
-       unsigned _M_v6;
-       in_addr _M_v4;
-      } _M_value;
+    private:
+      in_addr _M_v4;
+      unsigned _M_v6 = 0;
     };
 
     /// Set the default number of hops (TTL) for outbound datagrams.
-    struct hops : __sockopt_crtp<hops>
+    class hops : public __sockopt_crtp<hops>
     {
+    public:
       using __sockopt_crtp::__sockopt_crtp;
+      using __sockopt_crtp::operator=;
 
       template<typename _Protocol>
        int
@@ -2364,9 +2349,11 @@ namespace ip
     };
 
     /// Set whether datagrams are delivered back to the local application.
-    struct enable_loopback : __sockopt_crtp<enable_loopback>
+    class enable_loopback : public __sockopt_crtp<enable_loopback, bool>
     {
+    public:
       using __sockopt_crtp::__sockopt_crtp;
+      using __sockopt_crtp::operator=;
 
       template<typename _Protocol>
        int
index 538dc78..18849f6 100644 (file)
@@ -419,6 +419,8 @@ inline namespace v1
     noexcept
   { return __f1 = (__f1 ^ __f2); }
 
+  // TODO define socket_base static constants in .so for C++14 mode
+
 #if _GLIBCXX_HAVE_UNISTD_H
 
   class __socket_impl
diff --git a/libstdc++-v3/testsuite/experimental/net/internet/socket/opt.cc b/libstdc++-v3/testsuite/experimental/net/internet/socket/opt.cc
new file mode 100644 (file)
index 0000000..68bac84
--- /dev/null
@@ -0,0 +1,151 @@
+// { dg-do run { target c++14 } }
+
+#include <experimental/internet>
+#include <testsuite_common_types.h>
+#include <testsuite_hooks.h>
+
+using namespace std;
+
+template<typename C, typename T, typename P>
+void check_gettable_sockopt(P p, C c = C())
+{
+  static_assert( is_same<decltype(declval<const C&>().level(p)), int>(), "" );
+  static_assert( noexcept(declval<const C&>().level(p)), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().name(p)), int>(), "" );
+  static_assert( noexcept(declval<const C&>().name(p)), "" );
+
+  static_assert( is_same<decltype(declval<C&>().data(p)), void*>(), "" );
+  static_assert( noexcept(declval<C&>().data(p)), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().size(p)), size_t>(), "" );
+  static_assert( noexcept(declval<const C&>().size(p)), "" );
+
+  static_assert( is_same<decltype(declval<C&>().resize(p, 0)), void>(), "" );
+  static_assert( ! noexcept(declval<C&>().resize(p, 0)), "" );
+
+  VERIFY(c.size(p) == sizeof(T));
+}
+
+template<typename C, typename T, typename P>
+void check_settable_sockopt(P p, C c = C())
+{
+  static_assert( is_same<decltype(declval<const C&>().level(p)), int>(), "" );
+  static_assert( noexcept(declval<const C&>().level(p)), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().name(p)), int>(), "" );
+  static_assert( noexcept(declval<const C&>().name(p)), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().data(p)), const void*>(), "" );
+  static_assert( noexcept(declval<const C&>().data(p)), "" );
+
+  static_assert( is_same<decltype(declval<C&>().size(p)), size_t>(), "" );
+  static_assert( noexcept(declval<C&>().size(p)), "" );
+
+  VERIFY(c.size(p) == sizeof(T));
+}
+
+template<typename C, typename T = int>
+void check_boolean_sockopt()
+{
+  namespace ip = std::experimental::net::ip;
+  check_gettable_sockopt<C, T>(ip::tcp::v4());
+  check_settable_sockopt<C, T>(ip::tcp::v4());
+
+  static_assert( is_destructible<C>(), "" );
+  static_assert( is_nothrow_default_constructible<C>(), "" );
+  static_assert( is_nothrow_copy_constructible<C>(), "" );
+  static_assert( is_nothrow_copy_assignable<C>(), "" );
+
+  static_assert( is_nothrow_constructible<C, bool>(), "" );
+  static_assert( is_nothrow_assignable<C&, bool>(), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().value()), bool>(), "" );
+  static_assert( noexcept(declval<const C&>().value()), "" );
+
+  static_assert( is_same<decltype(static_cast<bool>(declval<const C&>())), bool>(), "" );
+  static_assert( noexcept(static_cast<bool>(declval<const C&>())), "" );
+
+  static_assert( is_same<decltype(!declval<const C&>()), bool>(), "" );
+  static_assert( noexcept(!declval<const C&>()), "" );
+}
+
+template<typename C, typename T = int>
+void check_integer_sockopt()
+{
+  namespace ip = std::experimental::net::ip;
+  check_gettable_sockopt<C, T>(ip::tcp::v4());
+  check_settable_sockopt<C, T>(ip::tcp::v4());
+
+  static_assert( is_destructible<C>(), "" );
+  static_assert( is_nothrow_default_constructible<C>(), "" );
+  static_assert( is_nothrow_copy_constructible<C>(), "" );
+  static_assert( is_nothrow_copy_assignable<C>(), "" );
+
+  static_assert( is_nothrow_constructible<C, int>(), "" );
+  static_assert( is_nothrow_assignable<C&, int>(), "" );
+
+  static_assert( is_same<decltype(declval<const C&>().value()), int>(), "" );
+  static_assert( noexcept(declval<const C&>().value()), "" );
+}
+
+template<typename C>
+void check_mcast_sockopt(C& c)
+{
+  namespace ip = std::experimental::net::ip;
+  static_assert( is_destructible<C>(), "" );
+  static_assert( is_copy_constructible<C>(), "" );
+  static_assert( is_copy_assignable<C>(), "" );
+
+  check_settable_sockopt<C, ipv6_mreq>(ip::tcp::v6(), c);
+  check_settable_sockopt<C, ip_mreq>(ip::tcp::v4(), c);
+
+  static_assert( is_nothrow_constructible<C, const ip::address&>(), "" );
+  static_assert( ! is_convertible<const ip::address&, C>(), "explicit" );
+  static_assert( is_nothrow_constructible<C, const ip::address_v4&>(), "" );
+  static_assert( ! is_convertible<const ip::address_v4&, C>(), "explicit" );
+  static_assert( is_nothrow_constructible<C, const ip::address_v4&, const ip::address_v4&>(), "" );
+  static_assert( is_nothrow_constructible<C, const ip::address_v6&>(), "" );
+  static_assert( ! is_convertible<const ip::address_v6&, C>(), "explicit" );
+  static_assert( is_nothrow_constructible<C, const ip::address_v6&, unsigned>(), "" );
+}
+
+void test_option_types()
+{
+  namespace ip = std::experimental::net::ip;
+#if __has_include(<netinet/in.h>)
+  check_boolean_sockopt<ip::tcp::no_delay>();
+
+  check_boolean_sockopt<ip::v6_only>();
+
+  check_integer_sockopt<ip::unicast::hops>();
+
+  ip::multicast::join_group join(ip::address_v4::any());
+  check_mcast_sockopt(join);
+
+  ip::multicast::leave_group leave(ip::address_v4::any());
+  check_mcast_sockopt(leave);
+
+  using outbound_if = ip::multicast::outbound_interface;
+  outbound_if oif(ip::address_v4::any());
+  check_settable_sockopt<outbound_if, unsigned>(ip::tcp::v6(), oif);
+  check_settable_sockopt<outbound_if, ::in_addr>(ip::tcp::v4(), oif);
+  static_assert( is_destructible<outbound_if>(), "" );
+  static_assert( ! is_default_constructible<outbound_if>(), "" );
+  static_assert( is_nothrow_copy_constructible<outbound_if>(), "" );
+  static_assert( is_nothrow_copy_assignable<outbound_if>(), "" );
+  static_assert( is_nothrow_constructible<outbound_if, const ip::address_v4&>(), "" );
+  static_assert( ! is_convertible<const ip::address_v4&, outbound_if>(), "explicit" );
+  static_assert( is_nothrow_constructible<outbound_if, unsigned>(), "" );
+  static_assert( ! is_convertible<unsigned, outbound_if>(), "explicit" );
+
+  check_integer_sockopt<ip::multicast::hops>();
+
+  check_boolean_sockopt<ip::multicast::enable_loopback>();
+#endif
+}
+
+int main()
+{
+  test_option_types();
+}
index 95cd815..1c02c5a 100644 (file)
 using S = std::experimental::net::socket_base;
 using namespace std;
 
+static_assert( ! is_default_constructible<S>(), "protected" );
+static_assert( ! is_destructible<S>(), "protected" );
+struct Sock : S { };
+static_assert( is_default_constructible<Sock>(), "" );
+static_assert( is_destructible<Sock>(), "" );
+
 // Dummy protocol
 struct P
 {
@@ -34,9 +40,6 @@ struct P
   };
 };
 
-static_assert( ! is_default_constructible<S>(), "" );
-static_assert( ! is_destructible<S>(), "" );
-
 template<typename C, typename T>
 void check_gettable_sockopt()
 {
@@ -92,6 +95,7 @@ void check_boolean_sockopt()
   static_assert( is_nothrow_copy_assignable<C>(), "" );
 
   static_assert( is_nothrow_constructible<C, bool>(), "" );
+  static_assert( ! is_convertible<bool, C>(), "constructor is explicit" );
   static_assert( is_nothrow_assignable<C&, bool>(), "" );
 
   static_assert( is_same<decltype(declval<const C&>().value()), bool>(), "" );
@@ -116,6 +120,7 @@ void check_integer_sockopt()
   static_assert( is_nothrow_copy_assignable<C>(), "" );
 
   static_assert( is_nothrow_constructible<C, int>(), "" );
+  static_assert( ! is_convertible<int, C>(), "constructor is explicit" );
   static_assert( is_nothrow_assignable<C&, int>(), "" );
 
   static_assert( is_same<decltype(declval<const C&>().value()), int>(), "" );
@@ -124,6 +129,7 @@ void check_integer_sockopt()
 
 void test_option_types()
 {
+#if __has_include(<socket.h>)
   check_boolean_sockopt<S::broadcast>();
 
   check_boolean_sockopt<S::debug>();
@@ -163,10 +169,12 @@ void test_option_types()
   check_integer_sockopt<S::send_buffer_size>();
 
   check_integer_sockopt<S::send_low_watermark>();
+#endif
 }
 
 void test_constants()
 {
+#if __has_include(<socket.h>)
   static_assert( is_enum<S::shutdown_type>::value, "" );
   static_assert( S::shutdown_receive != S::shutdown_send, "" );
   static_assert( S::shutdown_receive != S::shutdown_both, "" );
@@ -183,6 +191,7 @@ void test_constants()
 
   auto m = &S::max_listen_connections;
   static_assert( is_same<decltype(m), const int*>::value, "" );
+#endif
 }
 
 int main()