Merge pull request #9759 from michalpasztamobica/memory_handling_fixes

Cruz Monrreal · web-flow · commit b088bd493afa · 2019-02-20T12:07:45.000-06:00
Multiple memory handling fixes
diff --git a/UNITTESTS/features/netsocket/DTLSSocketWrapper/test_DTLSSocketWrapper.cpp b/UNITTESTS/features/netsocket/DTLSSocketWrapper/test_DTLSSocketWrapper.cpp
@@ -58,6 +58,7 @@ class TestDTLSSocketWrapper : public testing::Test {
         stack.return_socketAddress = SocketAddress();
         eventFlagsStubNextRetval.clear();
         delete wrapper;
+        delete transport;
     }
 
     char *cert = "-----BEGIN CERTIFICATE-----\
@@ -103,6 +104,7 @@ TEST_F(TestDTLSSocketWrapper, constructor)
 TEST_F(TestDTLSSocketWrapper, constructor_hostname)
 {
     DTLSSocketWrapper *wrapper2 = new DTLSSocketWrapper(transport, "localhost");
+    delete wrapper2;
 }
 
 /* connect */
@@ -323,6 +325,8 @@ TEST_F(TestDTLSSocketWrapper, set_root_ca_cert_invalid)
     mbedtls_stub.counter = 0;
     mbedtls_stub.retArray[0] = 1; // mbedtls_x509_crt_parse error
     EXPECT_EQ(wrapper->set_root_ca_cert(cert, strlen(cert)), NSAPI_ERROR_PARAMETER);
+    // We need to deallocate the crt pointer ourselves.
+    delete (wrapper->get_ca_chain());
 }
 
 TEST_F(TestDTLSSocketWrapper, set_client_cert_key)
diff --git a/UNITTESTS/features/netsocket/NetworkInterface/test_NetworkInterface.cpp b/UNITTESTS/features/netsocket/NetworkInterface/test_NetworkInterface.cpp
@@ -216,4 +216,6 @@ TEST_F(TestNetworkInterface, correct_event_listener_per_interface)
 
     iface->remove_event_listener(my_iface_callback);
     iface2->remove_event_listener(my_iface_callback2);
+
+    delete iface2;
 }
diff --git a/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp b/UNITTESTS/features/netsocket/TCPSocket/test_TCPSocket.cpp
@@ -272,8 +272,12 @@ TEST_F(TestTCPSocket, accept)
     nsapi_error_t error;
     stack.return_value = NSAPI_ERROR_OK;
     socket->open((NetworkStack *)&stack);
-    EXPECT_NE(socket->accept(&error), static_cast<TCPSocket *>(NULL));
+    TCPSocket *sock = socket->accept(&error);
+    EXPECT_NE(sock, static_cast<TCPSocket *>(NULL));
     EXPECT_EQ(error, NSAPI_ERROR_OK);
+    if (sock) {
+        sock->close();
+    }
 }
 
 TEST_F(TestTCPSocket, accept_would_block)
diff --git a/UNITTESTS/features/netsocket/TLSSocketWrapper/test_TLSSocketWrapper.cpp b/UNITTESTS/features/netsocket/TLSSocketWrapper/test_TLSSocketWrapper.cpp
@@ -56,6 +56,7 @@ class TestTLSSocketWrapper : public testing::Test {
         stack.return_values.clear();
         eventFlagsStubNextRetval.clear();
         delete wrapper;
+        delete transport;
     }
 
     char *cert = "-----BEGIN CERTIFICATE-----\
@@ -101,6 +102,7 @@ TEST_F(TestTLSSocketWrapper, constructor)
 TEST_F(TestTLSSocketWrapper, constructor_hostname)
 {
     TLSSocketWrapper *wrapper2 = new TLSSocketWrapper(transport, "localhost");
+    delete wrapper2;
 }
 
 /* connect */
diff --git a/UNITTESTS/features/netsocket/WiFiAccessPoint/test_WiFiAccessPoint.cpp b/UNITTESTS/features/netsocket/WiFiAccessPoint/test_WiFiAccessPoint.cpp
@@ -62,5 +62,6 @@ TEST_F(TestWiFiAccessPoint, set_data)
     EXPECT_EQ(testAp.channel, ap1->get_channel());
     EXPECT_EQ(testAp.rssi, ap1->get_rssi());
     EXPECT_EQ(testAp.security, ap1->get_security());
+    delete ap1;
 }
 
diff --git a/UNITTESTS/stubs/NetworkStack_stub.h b/UNITTESTS/stubs/NetworkStack_stub.h
@@ -27,9 +27,10 @@ class NetworkStackstub : public NetworkStack {
     nsapi_error_t return_value;
     SocketAddress return_socketAddress;
 
-    NetworkStackstub()
+    NetworkStackstub() :
+        return_value(0),
+        return_socketAddress()
     {
-        return_value = 0;
     }
 
     virtual const char *get_ip_address()
@@ -57,6 +58,8 @@ class NetworkStackstub : public NetworkStack {
         if (return_value == NSAPI_ERROR_OK && return_values.front() == NSAPI_ERROR_OK) {
             // Make sure a non-NULL value is returned if error is not expected
             *handle = reinterpret_cast<nsapi_socket_t *>(1234);
+        } else {
+            *handle = NULL;
         }
         return return_value;
     };
@@ -84,6 +87,12 @@ class NetworkStackstub : public NetworkStack {
     virtual nsapi_error_t socket_accept(nsapi_socket_t server,
                                         nsapi_socket_t *handle, SocketAddress *address = 0)
     {
+        if (return_value == NSAPI_ERROR_OK && return_values.front() == NSAPI_ERROR_OK) {
+            // Make sure a non-NULL value is returned if error is not expected
+            *handle = reinterpret_cast<nsapi_socket_t *>(1234);
+        } else {
+            *handle = NULL;
+        }
         return return_value;
     };
     virtual nsapi_size_or_error_t socket_send(nsapi_socket_t handle,
diff --git a/features/netsocket/InternetSocket.cpp b/features/netsocket/InternetSocket.cpp
@@ -21,6 +21,7 @@ using namespace mbed;
 
 InternetSocket::InternetSocket()
     : _stack(0), _socket(0), _timeout(osWaitForever),
+      _remote_peer(),
       _readers(0), _writers(0),
       _factory_allocated(false)
 {
diff --git a/features/netsocket/SocketAddress.cpp b/features/netsocket/SocketAddress.cpp
@@ -26,32 +26,43 @@
 
 SocketAddress::SocketAddress(nsapi_addr_t addr, uint16_t port)
 {
+    mem_init();
     _ip_address = NULL;
     set_addr(addr);
     set_port(port);
 }
 
 SocketAddress::SocketAddress(const char *addr, uint16_t port)
 {
+    mem_init();
     _ip_address = NULL;
     set_ip_address(addr);
     set_port(port);
 }
 
 SocketAddress::SocketAddress(const void *bytes, nsapi_version_t version, uint16_t port)
 {
+    mem_init();
     _ip_address = NULL;
     set_ip_bytes(bytes, version);
     set_port(port);
 }
 
 SocketAddress::SocketAddress(const SocketAddress &addr)
 {
+    mem_init();
     _ip_address = NULL;
     set_addr(addr.get_addr());
     set_port(addr.get_port());
 }
 
+void SocketAddress::mem_init(void)
+{
+    _addr.version = NSAPI_UNSPEC;
+    memset(_addr.bytes, 0, NSAPI_IP_BYTES);
+    _port = 0;
+}
+
 bool SocketAddress::set_ip_address(const char *addr)
 {
     delete[] _ip_address;
diff --git a/features/netsocket/SocketAddress.h b/features/netsocket/SocketAddress.h
@@ -178,6 +178,9 @@ class SocketAddress {
 private:
     void _SocketAddress(NetworkStack *iface, const char *host, uint16_t port);
 
+    /** Initialize memory */
+    void mem_init(void);
+
     mutable char *_ip_address;
     nsapi_addr_t _addr;
     uint16_t _port;
diff --git a/features/netsocket/TLSSocketWrapper.cpp b/features/netsocket/TLSSocketWrapper.cpp
@@ -109,6 +109,8 @@ nsapi_error_t TLSSocketWrapper::set_root_ca_cert(const void *root_ca, size_t len
     if ((ret = mbedtls_x509_crt_parse(crt, static_cast<const unsigned char *>(root_ca),
                                       len)) != 0) {
         print_mbedtls_error("mbedtls_x509_crt_parse", ret);
+        mbedtls_x509_crt_free(crt);
+        delete crt;
         return NSAPI_ERROR_PARAMETER;
     }
     set_ca_chain(crt);
@@ -140,12 +142,16 @@ nsapi_error_t TLSSocketWrapper::set_client_cert_key(const void *client_cert, siz
     if ((ret = mbedtls_x509_crt_parse(crt, static_cast<const unsigned char *>(client_cert),
                                       client_cert_len)) != 0) {
         print_mbedtls_error("mbedtls_x509_crt_parse", ret);
+        mbedtls_x509_crt_free(crt);
+        delete crt;
         return NSAPI_ERROR_PARAMETER;
     }
     mbedtls_pk_init(&_pkctx);
     if ((ret = mbedtls_pk_parse_key(&_pkctx, static_cast<const unsigned char *>(client_private_key_pem),
                                     client_private_key_len, NULL, 0)) != 0) {
         print_mbedtls_error("mbedtls_pk_parse_key", ret);
+        mbedtls_x509_crt_free(crt);
+        delete crt;
         return NSAPI_ERROR_PARAMETER;
     }
     set_own_cert(crt);

Original file line number	Diff line number	Diff line change
`@@ -58,6 +58,7 @@ class TestDTLSSocketWrapper : public testing::Test {`
`58`	`58`	`stack.return_socketAddress = SocketAddress();`
`59`	`59`	`eventFlagsStubNextRetval.clear();`
`60`	`60`	`delete wrapper;`
	`61`	`+ delete transport;`
`61`	`62`	`}`
`62`	`63`
`63`	`64`	`char *cert = "-----BEGIN CERTIFICATE-----\`
`@@ -103,6 +104,7 @@ TEST_F(TestDTLSSocketWrapper, constructor)`
`103`	`104`	`TEST_F(TestDTLSSocketWrapper, constructor_hostname)`
`104`	`105`	`{`
`105`	`106`	`DTLSSocketWrapper *wrapper2 = new DTLSSocketWrapper(transport, "localhost");`
	`107`	`+ delete wrapper2;`
`106`	`108`	`}`
`107`	`109`
`108`	`110`	`/* connect */`
`@@ -323,6 +325,8 @@ TEST_F(TestDTLSSocketWrapper, set_root_ca_cert_invalid)`
`323`	`325`	`mbedtls_stub.counter = 0;`
`324`	`326`	`mbedtls_stub.retArray[0] = 1; // mbedtls_x509_crt_parse error`
`325`	`327`	`EXPECT_EQ(wrapper->set_root_ca_cert(cert, strlen(cert)), NSAPI_ERROR_PARAMETER);`
	`328`	`+ // We need to deallocate the crt pointer ourselves.`
	`329`	`+ delete (wrapper->get_ca_chain());`
`326`	`330`	`}`
`327`	`331`
`328`	`332`	`TEST_F(TestDTLSSocketWrapper, set_client_cert_key)`
Original file line number	Diff line number	Diff line change
`@@ -216,4 +216,6 @@ TEST_F(TestNetworkInterface, correct_event_listener_per_interface)`
`216`	`216`
`217`	`217`	`iface->remove_event_listener(my_iface_callback);`
`218`	`218`	`iface2->remove_event_listener(my_iface_callback2);`
	`219`	`+`
	`220`	`+ delete iface2;`
`219`	`221`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,6 +56,7 @@ class TestTLSSocketWrapper : public testing::Test {`
`56`	`56`	`stack.return_values.clear();`
`57`	`57`	`eventFlagsStubNextRetval.clear();`
`58`	`58`	`delete wrapper;`
	`59`	`+ delete transport;`
`59`	`60`	`}`
`60`	`61`
`61`	`62`	`char *cert = "-----BEGIN CERTIFICATE-----\`
`@@ -101,6 +102,7 @@ TEST_F(TestTLSSocketWrapper, constructor)`
`101`	`102`	`TEST_F(TestTLSSocketWrapper, constructor_hostname)`
`102`	`103`	`{`
`103`	`104`	`TLSSocketWrapper *wrapper2 = new TLSSocketWrapper(transport, "localhost");`
	`105`	`+ delete wrapper2;`
`104`	`106`	`}`
`105`	`107`
`106`	`108`	`/* connect */`
Original file line number	Diff line number	Diff line change
`@@ -62,5 +62,6 @@ TEST_F(TestWiFiAccessPoint, set_data)`
`62`	`62`	`EXPECT_EQ(testAp.channel, ap1->get_channel());`
`63`	`63`	`EXPECT_EQ(testAp.rssi, ap1->get_rssi());`
`64`	`64`	`EXPECT_EQ(testAp.security, ap1->get_security());`
	`65`	`+ delete ap1;`
`65`	`66`	`}`
`66`	`67`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ using namespace mbed;`
`21`	`21`
`22`	`22`	`InternetSocket::InternetSocket()`
`23`	`23`	`: _stack(0), _socket(0), _timeout(osWaitForever),`
	`24`	`+ _remote_peer(),`
`24`	`25`	`_readers(0), _writers(0),`
`25`	`26`	`_factory_allocated(false)`
`26`	`27`	`{`
Original file line number	Diff line number	Diff line change
`@@ -26,32 +26,43 @@`
`26`	`26`
`27`	`27`	`SocketAddress::SocketAddress(nsapi_addr_t addr, uint16_t port)`
`28`	`28`	`{`
	`29`	`+ mem_init();`
`29`	`30`	`_ip_address = NULL;`
`30`	`31`	`set_addr(addr);`
`31`	`32`	`set_port(port);`
`32`	`33`	`}`
`33`	`34`
`34`	`35`	`SocketAddress::SocketAddress(const char *addr, uint16_t port)`
`35`	`36`	`{`
	`37`	`+ mem_init();`
`36`	`38`	`_ip_address = NULL;`
`37`	`39`	`set_ip_address(addr);`
`38`	`40`	`set_port(port);`
`39`	`41`	`}`
`40`	`42`
`41`	`43`	`SocketAddress::SocketAddress(const void *bytes, nsapi_version_t version, uint16_t port)`
`42`	`44`	`{`
	`45`	`+ mem_init();`
`43`	`46`	`_ip_address = NULL;`
`44`	`47`	`set_ip_bytes(bytes, version);`
`45`	`48`	`set_port(port);`
`46`	`49`	`}`
`47`	`50`
`48`	`51`	`SocketAddress::SocketAddress(const SocketAddress &addr)`
`49`	`52`	`{`
	`53`	`+ mem_init();`
`50`	`54`	`_ip_address = NULL;`
`51`	`55`	`set_addr(addr.get_addr());`
`52`	`56`	`set_port(addr.get_port());`
`53`	`57`	`}`
`54`	`58`
	`59`	`+void SocketAddress::mem_init(void)`
	`60`	`+{`
	`61`	`+ _addr.version = NSAPI_UNSPEC;`
	`62`	`+ memset(_addr.bytes, 0, NSAPI_IP_BYTES);`
	`63`	`+ _port = 0;`
	`64`	`+}`
	`65`	`+`
`55`	`66`	`bool SocketAddress::set_ip_address(const char *addr)`
`56`	`67`	`{`
`57`	`68`	`delete[] _ip_address;`