fix issue with freeing transport layer memory (#2066)

* fix issue with freeing transportlayer memory

 - Added helper function to reallocate memory.
 - Added unit test

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>

* rename function

UA_Server_addPubSubTransportLayer -> UA_ServerConfig_addPubSubTransportLayer

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>

* add documenation

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>

* move UA_ServerConfig_addPubSubTransportLayer

Move UA_ServerConfig_addPubSubTransportLayer to ua_config_default.c.

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>

include/ua_plugin_pubsub.h

@@ -85,6 +85,21 @@ typedef struct {
     UA_PubSubChannel * (*createPubSubChannel)(UA_PubSubConnectionConfig *connectionConfig);
 } UA_PubSubTransportLayer;
 
+
+/**
+ * The UA_ServerConfig_addPubSubTransportLayer is used to add a transport
+ * layer to the server configuration. The list memory is allocated and will be freed with
+ * UA_PubSubManager_delete.
+ *
+ * .. note:: If the UA_String transportProfileUri was dynamically allocated
+ *           the memory has to be freed when no longer required.
+ *
+ * .. note:: This has to be done before the server is started with UA_Server_run.
+ */
+UA_StatusCode
+UA_ServerConfig_addPubSubTransportLayer(UA_ServerConfig *config,
+        UA_PubSubTransportLayer *pubsubTransportLayer);
+
 #endif /* UA_ENABLE_PUBSUB */
 
 _UA_END_DECLS

plugins/ua_config_default.c

@@ -661,6 +661,37 @@ UA_ServerConfig_delete(UA_ServerConfig *config) {
     UA_free(config);
 }
 
+
+#ifdef UA_ENABLE_PUBSUB /* conditional compilation */
+/**
+ * Add a pubsubTransportLayer to the configuration.
+ * Memory is reallocated on demand */
+UA_StatusCode
+UA_ServerConfig_addPubSubTransportLayer(UA_ServerConfig *config,
+        UA_PubSubTransportLayer *pubsubTransportLayer) {
+
+    if(config->pubsubTransportLayersSize == 0) {
+        config->pubsubTransportLayers = (UA_PubSubTransportLayer *)
+                UA_malloc(sizeof(UA_PubSubTransportLayer));
+    } else {
+        config->pubsubTransportLayers = (UA_PubSubTransportLayer*)
+                UA_realloc(config->pubsubTransportLayers,
+                sizeof(UA_PubSubTransportLayer) * (config->pubsubTransportLayersSize + 1));
+    }
+
+    if (config->pubsubTransportLayers == NULL) {
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+    }
+
+    memcpy(&config->pubsubTransportLayers[config->pubsubTransportLayersSize],
+            pubsubTransportLayer, sizeof(UA_PubSubTransportLayer));
+    config->pubsubTransportLayersSize++;
+
+    return UA_STATUSCODE_GOOD;
+}
+#endif /* UA_ENABLE_PUBSUB */
+
+
 /***************************/
 /* Default Client Settings */
 /***************************/

src/pubsub/ua_pubsub_manager.c

@@ -281,9 +281,9 @@ void
 UA_PubSubManager_delete(UA_Server *server, UA_PubSubManager *pubSubManager) {
     UA_LOG_INFO(server->config.logger, UA_LOGCATEGORY_SERVER, "PubSub cleanup was called.");
     //free the currently configured transport layers
-    for(size_t i = 0; i < server->config.pubsubTransportLayersSize; i++){
-        UA_free(&server->config.pubsubTransportLayers[i]);
-    }
+    UA_free(server->config.pubsubTransportLayers);
+    server->config.pubsubTransportLayersSize = 0;
+
     //remove Connections and WriterGroups
     while(pubSubManager->connectionsSize > 0){
         UA_Server_removePubSubConnection(server, pubSubManager->connections[pubSubManager->connectionsSize-1].identifier);

tests/CMakeLists.txt

@@ -200,6 +200,11 @@ if(UA_ENABLE_PUBSUB)
     add_executable(check_pubsub_publish pubsub/check_pubsub_publish.c $<TARGET_OBJECTS:open62541-object> $<TARGET_OBJECTS:open62541-plugins>)
     target_link_libraries(check_pubsub_publish ${LIBS})
     add_test_valgrind(check_pubsub_publish ${TESTS_BINARY_DIR}/check_pubsub_publish)
+
+    add_executable(check_pubsub_multiple_layer pubsub/check_pubsub_multiple_layer.c $<TARGET_OBJECTS:open62541-object> $<TARGET_OBJECTS:open62541-plugins>)
+    target_link_libraries(check_pubsub_multiple_layer ${LIBS})
+    add_test_valgrind(check_pubsub_multiple_layer ${TESTS_BINARY_DIR}/check_pubsub_multiple_layer)
+
     if(UA_ENABLE_PUBSUB_INFORMATIONMODEL)
         add_executable(check_pubsub_informationmodel pubsub/check_pubsub_informationmodel.c $<TARGET_OBJECTS:open62541-object> $<TARGET_OBJECTS:open62541-plugins>)
         target_link_libraries(check_pubsub_informationmodel ${LIBS})

tests/pubsub/check_pubsub_multiple_layer.c

@@ -0,0 +1,69 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * Copyright (c) 2017 - 2018 Fraunhofer IOSB (Author: Andreas Ebner)
+ */
+
+#include "ua_server_pubsub.h"
+#include "src_generated/ua_types_generated_encoding_binary.h"
+#include "ua_types.h"
+#include "ua_pubsub.h"
+#include "ua_config_default.h"
+#include "ua_network_pubsub_udp.h"
+#include "ua_server_internal.h"
+#include "check.h"
+#include "stdio.h"
+
+UA_Server *server = NULL;
+UA_ServerConfig *config = NULL;
+UA_NodeId connection1, connection2;
+
+static void setup(void) {
+}
+
+static void teardown(void) {
+}
+
+START_TEST(AddMultipleTransportLayers){
+    config = UA_ServerConfig_new_default();
+
+    UA_PubSubTransportLayer pubsubTransportLayer;
+
+    ck_assert_int_eq(config->pubsubTransportLayersSize, 0);
+
+    pubsubTransportLayer = UA_PubSubTransportLayerUDPMP();
+    UA_ServerConfig_addPubSubTransportLayer(config, &pubsubTransportLayer);
+    ck_assert_int_eq(config->pubsubTransportLayersSize, 1);
+
+    pubsubTransportLayer = UA_PubSubTransportLayerUDPMP();
+    UA_ServerConfig_addPubSubTransportLayer(config, &pubsubTransportLayer);
+    ck_assert_int_eq(config->pubsubTransportLayersSize, 2);
+
+    pubsubTransportLayer = UA_PubSubTransportLayerUDPMP();
+    UA_ServerConfig_addPubSubTransportLayer(config, &pubsubTransportLayer);
+    ck_assert_int_eq(config->pubsubTransportLayersSize, 3);
+
+    server = UA_Server_new(config);
+    UA_Server_delete(server);
+
+    UA_ServerConfig_delete(config);
+
+    } END_TEST
+
+
+int main(void) {
+    TCase *tc_add_pubsub_writergroup = tcase_create("PubSub Layer allocation");
+    tcase_add_checked_fixture(tc_add_pubsub_writergroup, setup, teardown);
+    tcase_add_test(tc_add_pubsub_writergroup, AddMultipleTransportLayers);
+
+    Suite *s = suite_create("PubSub Multiple layer handling");
+    suite_add_tcase(s, tc_add_pubsub_writergroup);
+
+    SRunner *sr = srunner_create(s);
+    srunner_set_fork_status(sr, CK_NOFORK);
+    srunner_run_all(sr,CK_NORMAL);
+    int number_failed = srunner_ntests_failed(sr);
+    srunner_free(sr);
+    return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
+}