From 70a6701006ad37ce0e02e8dd9c53f791df520c93 Mon Sep 17 00:00:00 2001 From: Kevin Bracey Date: Tue, 25 Feb 2020 17:18:21 +0200 Subject: [PATCH] Improve nsapi_create_stack Use tag dispatch to better handle both NetworkInterface and NetworkStack pointers. The previous design was intended to avoid ambiguities when presented with a scenario like class MyDevice : public NetworkInterface, public NetworkStack { }; TCPSocket(&MyDevice); // Need NetworkStack *: use NetworkInterface::get_stack or // cast to NetworkStack? But the previous solution didn't actually work as intended. The overload pair nsapi_create_stack(NetworkStack *); // versus template nsapi_create_stack(IF *); would only select the first form if passed an exact match - `NetworkStack *`. If passed a derived class pointer, like `MyDevice *`, it would select the template. This meant that an ambiguity for MyDevice was at least avoided, but in the wrong direction, potentially increasing code size. But in other cases, the system just didn't work at all - you couldn't pass a `MyStack *` pointer, unless you cast it to `NetworkStack *`. Quite a few bits of test code do this. Add a small bit of tag dispatch to prioritise the cast whenever the supplied pointer is convertible to `NetworkStack *`. --- .../netsocket/TCPSocket/test_TCPSocket.cpp | 46 +++++++++---------- features/netsocket/NetworkInterface.h | 4 +- features/netsocket/NetworkStack.h | 18 +++++--- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp b/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp index a49c7809fa..bf42d51472 100644 --- a/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp +++ b/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp @@ -62,8 +62,8 @@ TEST_F(TestTCPSocket, constructor) TEST_F(TestTCPSocket, constructor_parameters) { - TCPSocket socketParam = TCPSocket(); - socketParam.open(dynamic_cast(&stack)); + TCPSocket socketParam; + socketParam.open(&stack); const SocketAddress a("127.0.0.1", 1024); EXPECT_EQ(socketParam.connect(a), NSAPI_ERROR_OK); } @@ -72,7 +72,7 @@ TEST_F(TestTCPSocket, constructor_parameters) TEST_F(TestTCPSocket, connect) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_OK; const SocketAddress a("127.0.0.1", 1024); @@ -90,7 +90,7 @@ TEST_F(TestTCPSocket, connect_no_open) TEST_F(TestTCPSocket, connect_error_in_progress_no_timeout) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_IN_PROGRESS; const SocketAddress a("127.0.0.1", 1024); eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop @@ -99,7 +99,7 @@ TEST_F(TestTCPSocket, connect_error_in_progress_no_timeout) TEST_F(TestTCPSocket, connect_with_timeout) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_IN_PROGRESS; const SocketAddress a("127.0.0.1", 1024); eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop @@ -109,7 +109,7 @@ TEST_F(TestTCPSocket, connect_with_timeout) TEST_F(TestTCPSocket, connect_error_is_connected) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_values.push_back(NSAPI_ERROR_IS_CONNECTED); stack.return_values.push_back(NSAPI_ERROR_ALREADY); const SocketAddress a("127.0.0.1", 1024); @@ -128,14 +128,14 @@ TEST_F(TestTCPSocket, send_no_open) TEST_F(TestTCPSocket, send_in_one_chunk) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = dataSize; EXPECT_EQ(socket->send(dataBuf, dataSize), dataSize); } TEST_F(TestTCPSocket, send_in_two_chunks) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_values.push_back(4); stack.return_values.push_back(dataSize - 4); EXPECT_EQ(socket->send(dataBuf, dataSize), dataSize); @@ -143,7 +143,7 @@ TEST_F(TestTCPSocket, send_in_two_chunks) TEST_F(TestTCPSocket, send_error_would_block) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_WOULD_BLOCK; eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_WOULD_BLOCK); @@ -151,14 +151,14 @@ TEST_F(TestTCPSocket, send_error_would_block) TEST_F(TestTCPSocket, send_error_other) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_NO_MEMORY; EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_NO_MEMORY); } TEST_F(TestTCPSocket, send_error_no_timeout) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_WOULD_BLOCK; socket->set_blocking(false); EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_WOULD_BLOCK); @@ -166,7 +166,7 @@ TEST_F(TestTCPSocket, send_error_no_timeout) TEST_F(TestTCPSocket, send_to) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = 10; const SocketAddress a("127.0.0.1", 1024); EXPECT_EQ(socket->sendto(a, dataBuf, dataSize), dataSize); @@ -182,14 +182,14 @@ TEST_F(TestTCPSocket, recv_no_open) TEST_F(TestTCPSocket, recv_all_data) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = dataSize; EXPECT_EQ(socket->recv(dataBuf, dataSize), dataSize); } TEST_F(TestTCPSocket, recv_less_than_expected) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); unsigned int lessThanDataSize = dataSize - 1; stack.return_values.push_back(lessThanDataSize); EXPECT_EQ(socket->recv(dataBuf, dataSize), lessThanDataSize); @@ -197,7 +197,7 @@ TEST_F(TestTCPSocket, recv_less_than_expected) TEST_F(TestTCPSocket, recv_would_block) { - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_WOULD_BLOCK; eventFlagsStubNextRetval.push_back(0); eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop @@ -215,7 +215,7 @@ TEST_F(TestTCPSocket, recv_from) { stack.return_value = NSAPI_ERROR_OK; SocketAddress a("127.0.0.1", 1024); - EXPECT_EQ(socket->open((NetworkStack *)&stack), NSAPI_ERROR_OK); + EXPECT_EQ(socket->open(&stack), NSAPI_ERROR_OK); EXPECT_EQ(socket->connect(a), NSAPI_ERROR_OK); SocketAddress b; EXPECT_EQ(socket->recvfrom(&b, dataBuf, dataSize), NSAPI_ERROR_OK); @@ -226,7 +226,7 @@ TEST_F(TestTCPSocket, recv_from_null) { stack.return_value = NSAPI_ERROR_OK; SocketAddress a("127.0.0.1", 1024); - EXPECT_EQ(socket->open((NetworkStack *)&stack), NSAPI_ERROR_OK); + EXPECT_EQ(socket->open(&stack), NSAPI_ERROR_OK); EXPECT_EQ(socket->connect(a), NSAPI_ERROR_OK); EXPECT_EQ(socket->recvfrom(NULL, dataBuf, dataSize), NSAPI_ERROR_OK); } @@ -242,7 +242,7 @@ TEST_F(TestTCPSocket, listen_no_open) TEST_F(TestTCPSocket, listen) { stack.return_value = NSAPI_ERROR_OK; - socket->open((NetworkStack *)&stack); + socket->open(&stack); EXPECT_EQ(socket->listen(1), NSAPI_ERROR_OK); } @@ -252,7 +252,7 @@ TEST_F(TestTCPSocket, accept_no_open) { nsapi_error_t error; stack.return_value = NSAPI_ERROR_OK; - EXPECT_EQ(socket->accept(&error), static_cast(NULL)); + EXPECT_EQ(socket->accept(&error), nullptr); EXPECT_EQ(error, NSAPI_ERROR_NO_SOCKET); } @@ -260,9 +260,9 @@ TEST_F(TestTCPSocket, accept) { nsapi_error_t error; stack.return_value = NSAPI_ERROR_OK; - socket->open((NetworkStack *)&stack); + socket->open(&stack); TCPSocket *sock = socket->accept(&error); - EXPECT_NE(sock, static_cast(NULL)); + EXPECT_NE(sock, nullptr); EXPECT_EQ(error, NSAPI_ERROR_OK); if (sock) { sock->close(); @@ -272,11 +272,11 @@ TEST_F(TestTCPSocket, accept) TEST_F(TestTCPSocket, accept_would_block) { nsapi_error_t error; - socket->open((NetworkStack *)&stack); + socket->open(&stack); stack.return_value = NSAPI_ERROR_WOULD_BLOCK; eventFlagsStubNextRetval.push_back(0); eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop - EXPECT_EQ(socket->accept(&error), static_cast(NULL)); + EXPECT_EQ(socket->accept(&error), nullptr); EXPECT_EQ(error, NSAPI_ERROR_WOULD_BLOCK); } diff --git a/features/netsocket/NetworkInterface.h b/features/netsocket/NetworkInterface.h index 5f075a42dd..3a624e9238 100644 --- a/features/netsocket/NetworkInterface.h +++ b/features/netsocket/NetworkInterface.h @@ -426,8 +426,8 @@ protected: friend class TCPSocket; friend class TCPServer; friend class SocketAddress; - template - friend NetworkStack *nsapi_create_stack(IF *iface); + + friend NetworkStack *_nsapi_create_stack(NetworkInterface *iface, std::false_type); /** Provide access to the NetworkStack object * diff --git a/features/netsocket/NetworkStack.h b/features/netsocket/NetworkStack.h index d57c0664e5..f1f693d775 100644 --- a/features/netsocket/NetworkStack.h +++ b/features/netsocket/NetworkStack.h @@ -18,6 +18,7 @@ #ifndef NETWORK_STACK_H #define NETWORK_STACK_H +#include #include "nsapi_types.h" #include "netsocket/SocketAddress.h" #include "netsocket/NetworkInterface.h" @@ -475,6 +476,16 @@ private: virtual nsapi_error_t call_in(int delay, mbed::Callback func); }; +inline NetworkStack *_nsapi_create_stack(NetworkStack *stack, std::true_type /* convertible to NetworkStack */) +{ + return stack; +} + +inline NetworkStack *_nsapi_create_stack(NetworkInterface *iface, std::false_type /* not convertible to NetworkStack */) +{ + return iface->get_stack(); +} + /** Convert a raw nsapi_stack_t object into a C++ NetworkStack object * * @param stack Pointer to an object that can be converted to a stack @@ -485,15 +496,10 @@ private: */ NetworkStack *nsapi_create_stack(nsapi_stack_t *stack); -inline NetworkStack *nsapi_create_stack(NetworkStack *stack) -{ - return stack; -} - template NetworkStack *nsapi_create_stack(IF *iface) { - return nsapi_create_stack(static_cast(iface)->get_stack()); + return _nsapi_create_stack(iface, std::is_convertible()); }