diff mbox series

[committed,2/2] libstdc++: Fix internet socket option classes

Message ID 20210426202206.GK3008@redhat.com
State New
Headers show
Series [committed,1/2] libstdc++: Fix socket option classes | expand

Commit Message

Jonathan Wakely April 26, 2021, 8:22 p.m. UTC
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.



Tested powerpc64le-linux and powerpc-aix. Committed to trunk.
diff mbox series

Patch

commit 2e0b1c6ce3afe0670b96444c6b955ce184ed0046
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Apr 26 21:16:21 2021

    libstdc++: Fix internet socket option classes
    
    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.

diff --git a/libstdc++-v3/include/experimental/internet b/libstdc++-v3/include/experimental/internet
index d3321afb9c6..2e3fd06ead2 100644
--- a/libstdc++-v3/include/experimental/internet
+++ b/libstdc++-v3/include/experimental/internet
@@ -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,130 +2193,111 @@  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
 	level(const _Protocol& __p) const noexcept
 	{ 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);
-	}
+	{ return __p.family() == AF_INET6 ? sizeof(_M_v6) : sizeof(_M_v4); }
+
+    private:
+      ipv6_mreq _M_v6 = {};
+      ip_mreq _M_v4 = {};
+    };
+
+    /// Request that a socket joins a multicast group.
+    class join_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_JOIN_GROUP;
+	  return IP_ADD_MEMBERSHIP;
 	}
-
-    protected:
-      union
-      {
-	ipv6_mreq _M_v6;
-	ip_mreq _M_v4;
-      } _M_value;
     };
 
     /// Request that a socket leaves a multicast group.
-    struct leave_group
+    class leave_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_LEAVE_GROUP;
+	  return IP_DROP_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);
-	}
-
-      template<typename _Protocol>
-	void
-	resize(const _Protocol& __p, size_t __s)
-	{
-	  if (__s != size(__p))
-	    __throw_length_error("invalid value for socket option resize");
-	}
-
-    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
diff --git a/libstdc++-v3/include/experimental/socket b/libstdc++-v3/include/experimental/socket
index 538dc78e72c..18849f607f8 100644
--- a/libstdc++-v3/include/experimental/socket
+++ b/libstdc++-v3/include/experimental/socket
@@ -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
index 00000000000..68bac84a8b1
--- /dev/null
+++ b/libstdc++-v3/testsuite/experimental/net/internet/socket/opt.cc
@@ -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();
+}
diff --git a/libstdc++-v3/testsuite/experimental/net/socket/socket_base.cc b/libstdc++-v3/testsuite/experimental/net/socket/socket_base.cc
index 95cd8151840..1c02c5a09da 100644
--- a/libstdc++-v3/testsuite/experimental/net/socket/socket_base.cc
+++ b/libstdc++-v3/testsuite/experimental/net/socket/socket_base.cc
@@ -24,6 +24,12 @@ 
 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()