Skip to content

Commit 70a6701

Browse files
committed
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 <class IF> 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 *`.
1 parent aa6fd58 commit 70a6701

File tree

3 files changed

+37
-31
lines changed

3 files changed

+37
-31
lines changed

UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ TEST_F(TestTCPSocket, constructor)
6262

6363
TEST_F(TestTCPSocket, constructor_parameters)
6464
{
65-
TCPSocket socketParam = TCPSocket();
66-
socketParam.open(dynamic_cast<NetworkStack *>(&stack));
65+
TCPSocket socketParam;
66+
socketParam.open(&stack);
6767
const SocketAddress a("127.0.0.1", 1024);
6868
EXPECT_EQ(socketParam.connect(a), NSAPI_ERROR_OK);
6969
}
@@ -72,7 +72,7 @@ TEST_F(TestTCPSocket, constructor_parameters)
7272

7373
TEST_F(TestTCPSocket, connect)
7474
{
75-
socket->open((NetworkStack *)&stack);
75+
socket->open(&stack);
7676

7777
stack.return_value = NSAPI_ERROR_OK;
7878
const SocketAddress a("127.0.0.1", 1024);
@@ -90,7 +90,7 @@ TEST_F(TestTCPSocket, connect_no_open)
9090

9191
TEST_F(TestTCPSocket, connect_error_in_progress_no_timeout)
9292
{
93-
socket->open((NetworkStack *)&stack);
93+
socket->open(&stack);
9494
stack.return_value = NSAPI_ERROR_IN_PROGRESS;
9595
const SocketAddress a("127.0.0.1", 1024);
9696
eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop
@@ -99,7 +99,7 @@ TEST_F(TestTCPSocket, connect_error_in_progress_no_timeout)
9999

100100
TEST_F(TestTCPSocket, connect_with_timeout)
101101
{
102-
socket->open((NetworkStack *)&stack);
102+
socket->open(&stack);
103103
stack.return_value = NSAPI_ERROR_IN_PROGRESS;
104104
const SocketAddress a("127.0.0.1", 1024);
105105
eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop
@@ -109,7 +109,7 @@ TEST_F(TestTCPSocket, connect_with_timeout)
109109

110110
TEST_F(TestTCPSocket, connect_error_is_connected)
111111
{
112-
socket->open((NetworkStack *)&stack);
112+
socket->open(&stack);
113113
stack.return_values.push_back(NSAPI_ERROR_IS_CONNECTED);
114114
stack.return_values.push_back(NSAPI_ERROR_ALREADY);
115115
const SocketAddress a("127.0.0.1", 1024);
@@ -128,45 +128,45 @@ TEST_F(TestTCPSocket, send_no_open)
128128

129129
TEST_F(TestTCPSocket, send_in_one_chunk)
130130
{
131-
socket->open((NetworkStack *)&stack);
131+
socket->open(&stack);
132132
stack.return_value = dataSize;
133133
EXPECT_EQ(socket->send(dataBuf, dataSize), dataSize);
134134
}
135135

136136
TEST_F(TestTCPSocket, send_in_two_chunks)
137137
{
138-
socket->open((NetworkStack *)&stack);
138+
socket->open(&stack);
139139
stack.return_values.push_back(4);
140140
stack.return_values.push_back(dataSize - 4);
141141
EXPECT_EQ(socket->send(dataBuf, dataSize), dataSize);
142142
}
143143

144144
TEST_F(TestTCPSocket, send_error_would_block)
145145
{
146-
socket->open((NetworkStack *)&stack);
146+
socket->open(&stack);
147147
stack.return_value = NSAPI_ERROR_WOULD_BLOCK;
148148
eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop
149149
EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_WOULD_BLOCK);
150150
}
151151

152152
TEST_F(TestTCPSocket, send_error_other)
153153
{
154-
socket->open((NetworkStack *)&stack);
154+
socket->open(&stack);
155155
stack.return_value = NSAPI_ERROR_NO_MEMORY;
156156
EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_NO_MEMORY);
157157
}
158158

159159
TEST_F(TestTCPSocket, send_error_no_timeout)
160160
{
161-
socket->open((NetworkStack *)&stack);
161+
socket->open(&stack);
162162
stack.return_value = NSAPI_ERROR_WOULD_BLOCK;
163163
socket->set_blocking(false);
164164
EXPECT_EQ(socket->send(dataBuf, dataSize), NSAPI_ERROR_WOULD_BLOCK);
165165
}
166166

167167
TEST_F(TestTCPSocket, send_to)
168168
{
169-
socket->open((NetworkStack *)&stack);
169+
socket->open(&stack);
170170
stack.return_value = 10;
171171
const SocketAddress a("127.0.0.1", 1024);
172172
EXPECT_EQ(socket->sendto(a, dataBuf, dataSize), dataSize);
@@ -182,22 +182,22 @@ TEST_F(TestTCPSocket, recv_no_open)
182182

183183
TEST_F(TestTCPSocket, recv_all_data)
184184
{
185-
socket->open((NetworkStack *)&stack);
185+
socket->open(&stack);
186186
stack.return_value = dataSize;
187187
EXPECT_EQ(socket->recv(dataBuf, dataSize), dataSize);
188188
}
189189

190190
TEST_F(TestTCPSocket, recv_less_than_expected)
191191
{
192-
socket->open((NetworkStack *)&stack);
192+
socket->open(&stack);
193193
unsigned int lessThanDataSize = dataSize - 1;
194194
stack.return_values.push_back(lessThanDataSize);
195195
EXPECT_EQ(socket->recv(dataBuf, dataSize), lessThanDataSize);
196196
}
197197

198198
TEST_F(TestTCPSocket, recv_would_block)
199199
{
200-
socket->open((NetworkStack *)&stack);
200+
socket->open(&stack);
201201
stack.return_value = NSAPI_ERROR_WOULD_BLOCK;
202202
eventFlagsStubNextRetval.push_back(0);
203203
eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop
@@ -215,7 +215,7 @@ TEST_F(TestTCPSocket, recv_from)
215215
{
216216
stack.return_value = NSAPI_ERROR_OK;
217217
SocketAddress a("127.0.0.1", 1024);
218-
EXPECT_EQ(socket->open((NetworkStack *)&stack), NSAPI_ERROR_OK);
218+
EXPECT_EQ(socket->open(&stack), NSAPI_ERROR_OK);
219219
EXPECT_EQ(socket->connect(a), NSAPI_ERROR_OK);
220220
SocketAddress b;
221221
EXPECT_EQ(socket->recvfrom(&b, dataBuf, dataSize), NSAPI_ERROR_OK);
@@ -226,7 +226,7 @@ TEST_F(TestTCPSocket, recv_from_null)
226226
{
227227
stack.return_value = NSAPI_ERROR_OK;
228228
SocketAddress a("127.0.0.1", 1024);
229-
EXPECT_EQ(socket->open((NetworkStack *)&stack), NSAPI_ERROR_OK);
229+
EXPECT_EQ(socket->open(&stack), NSAPI_ERROR_OK);
230230
EXPECT_EQ(socket->connect(a), NSAPI_ERROR_OK);
231231
EXPECT_EQ(socket->recvfrom(NULL, dataBuf, dataSize), NSAPI_ERROR_OK);
232232
}
@@ -242,7 +242,7 @@ TEST_F(TestTCPSocket, listen_no_open)
242242
TEST_F(TestTCPSocket, listen)
243243
{
244244
stack.return_value = NSAPI_ERROR_OK;
245-
socket->open((NetworkStack *)&stack);
245+
socket->open(&stack);
246246
EXPECT_EQ(socket->listen(1), NSAPI_ERROR_OK);
247247
}
248248

@@ -252,17 +252,17 @@ TEST_F(TestTCPSocket, accept_no_open)
252252
{
253253
nsapi_error_t error;
254254
stack.return_value = NSAPI_ERROR_OK;
255-
EXPECT_EQ(socket->accept(&error), static_cast<TCPSocket *>(NULL));
255+
EXPECT_EQ(socket->accept(&error), nullptr);
256256
EXPECT_EQ(error, NSAPI_ERROR_NO_SOCKET);
257257
}
258258

259259
TEST_F(TestTCPSocket, accept)
260260
{
261261
nsapi_error_t error;
262262
stack.return_value = NSAPI_ERROR_OK;
263-
socket->open((NetworkStack *)&stack);
263+
socket->open(&stack);
264264
TCPSocket *sock = socket->accept(&error);
265-
EXPECT_NE(sock, static_cast<TCPSocket *>(NULL));
265+
EXPECT_NE(sock, nullptr);
266266
EXPECT_EQ(error, NSAPI_ERROR_OK);
267267
if (sock) {
268268
sock->close();
@@ -272,11 +272,11 @@ TEST_F(TestTCPSocket, accept)
272272
TEST_F(TestTCPSocket, accept_would_block)
273273
{
274274
nsapi_error_t error;
275-
socket->open((NetworkStack *)&stack);
275+
socket->open(&stack);
276276
stack.return_value = NSAPI_ERROR_WOULD_BLOCK;
277277
eventFlagsStubNextRetval.push_back(0);
278278
eventFlagsStubNextRetval.push_back(osFlagsError); // Break the wait loop
279-
EXPECT_EQ(socket->accept(&error), static_cast<TCPSocket *>(NULL));
279+
EXPECT_EQ(socket->accept(&error), nullptr);
280280
EXPECT_EQ(error, NSAPI_ERROR_WOULD_BLOCK);
281281
}
282282

features/netsocket/NetworkInterface.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -426,8 +426,8 @@ class NetworkInterface: public DNS {
426426
friend class TCPSocket;
427427
friend class TCPServer;
428428
friend class SocketAddress;
429-
template <typename IF>
430-
friend NetworkStack *nsapi_create_stack(IF *iface);
429+
430+
friend NetworkStack *_nsapi_create_stack(NetworkInterface *iface, std::false_type);
431431

432432
/** Provide access to the NetworkStack object
433433
*

features/netsocket/NetworkStack.h

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#ifndef NETWORK_STACK_H
1919
#define NETWORK_STACK_H
2020

21+
#include <type_traits>
2122
#include "nsapi_types.h"
2223
#include "netsocket/SocketAddress.h"
2324
#include "netsocket/NetworkInterface.h"
@@ -475,6 +476,16 @@ class NetworkStack : public DNS {
475476
virtual nsapi_error_t call_in(int delay, mbed::Callback<void()> func);
476477
};
477478

479+
inline NetworkStack *_nsapi_create_stack(NetworkStack *stack, std::true_type /* convertible to NetworkStack */)
480+
{
481+
return stack;
482+
}
483+
484+
inline NetworkStack *_nsapi_create_stack(NetworkInterface *iface, std::false_type /* not convertible to NetworkStack */)
485+
{
486+
return iface->get_stack();
487+
}
488+
478489
/** Convert a raw nsapi_stack_t object into a C++ NetworkStack object
479490
*
480491
* @param stack Pointer to an object that can be converted to a stack
@@ -485,15 +496,10 @@ class NetworkStack : public DNS {
485496
*/
486497
NetworkStack *nsapi_create_stack(nsapi_stack_t *stack);
487498

488-
inline NetworkStack *nsapi_create_stack(NetworkStack *stack)
489-
{
490-
return stack;
491-
}
492-
493499
template <typename IF>
494500
NetworkStack *nsapi_create_stack(IF *iface)
495501
{
496-
return nsapi_create_stack(static_cast<NetworkInterface *>(iface)->get_stack());
502+
return _nsapi_create_stack(iface, std::is_convertible<IF *, NetworkStack *>());
497503
}
498504

499505

0 commit comments

Comments
 (0)