Browse Source

fix(server): copy periodic server register url to avoid memory leaks

Stefan Profanter 5 years ago
parent
commit
30fdf2a71e

+ 2 - 2
include/open62541/server.h

@@ -547,8 +547,8 @@ UA_Server_unregister_discovery(UA_Server *server, struct UA_Client *client);
   * @param client the client which is used to call the RegisterServer.
   *         It must not yet be connected and will be connected for every register call
   *         to the given discoveryServerUrl.
-  * @param discoveryServerUrl if set to NULL, the default value
-  *        'opc.tcp://localhost:4840' will be used
+  * @param discoveryServerUrl where this server should register itself.
+  *        The string will be copied internally. Therefore you can free it after calling this method.
   * @param intervalMs
   * @param delayFirstRegisterMs
   * @param periodicCallbackId */

+ 2 - 0
src/server/ua_discovery_manager.c

@@ -162,6 +162,8 @@ UA_DiscoveryManager_deleteMembers(UA_DiscoveryManager *dm, UA_Server *server) {
     periodicServerRegisterCallback_entry *ps, *ps_tmp;
     LIST_FOREACH_SAFE(ps, &dm->periodicServerRegisterCallbacks, pointers, ps_tmp) {
         LIST_REMOVE(ps, pointers);
+        if (ps->callback->discovery_server_url)
+            UA_free(ps->callback->discovery_server_url);
         UA_free(ps->callback);
         UA_free(ps);
     }

+ 9 - 0
src/server/ua_discovery_manager.h

@@ -31,6 +31,15 @@ typedef struct registeredServer_list_entry {
     UA_DateTime lastSeen;
 } registeredServer_list_entry;
 
+struct PeriodicServerRegisterCallback {
+    UA_UInt64 id;
+    UA_Double this_interval;
+    UA_Double default_interval;
+    UA_Boolean registered;
+    struct UA_Client* client;
+    char* discovery_server_url;
+};
+
 typedef struct periodicServerRegisterCallback_entry {
 #ifdef UA_ENABLE_MULTITHREADING
     UA_DelayedCallback delayedCleanup;

+ 9 - 20
src/server/ua_services_discovery.c

@@ -545,15 +545,6 @@ void UA_Discovery_cleanupTimedOut(UA_Server *server, UA_DateTime nowMonotonic) {
     }
 }
 
-struct PeriodicServerRegisterCallback {
-    UA_UInt64 id;
-    UA_Double this_interval;
-    UA_Double default_interval;
-    UA_Boolean registered;
-    UA_Client* client;
-    const char* discovery_server_url;
-};
-
 /* Called by the UA_Server callback. The OPC UA specification says:
  *
  * > If an error occurs during registration (e.g. the Discovery Server is not running) then the Server
@@ -569,14 +560,7 @@ periodicServerRegister(UA_Server *server, void *data) {
 
     struct PeriodicServerRegisterCallback *cb = (struct PeriodicServerRegisterCallback *)data;
 
-    /* Which URL to register on */
-    // fixme: remove magic url
-    const char * server_url;
-    if(cb->discovery_server_url != NULL)
-        server_url = cb->discovery_server_url;
-    else
-        server_url = "opc.tcp://localhost:4840";
-    UA_StatusCode retval = UA_Client_connect_noSession(cb->client, server_url);
+    UA_StatusCode retval = UA_Client_connect_noSession(cb->client, cb->discovery_server_url);
     if (retval == UA_STATUSCODE_GOOD) {
         /* Register
            You can also use a semaphore file. That file must exist. When the file is
@@ -660,6 +644,7 @@ UA_Server_addPeriodicServerRegisterCallback(UA_Server *server,
                             "There is already a register callback for '%s' in place. Removing the older one.", discoveryServerUrl);
                 UA_Server_removeRepeatedCallback(server, rs->callback->id);
                 LIST_REMOVE(rs, pointers);
+                UA_free(rs->callback->discovery_server_url);
                 UA_free(rs->callback);
                 UA_free(rs);
                 break;
@@ -681,9 +666,13 @@ UA_Server_addPeriodicServerRegisterCallback(UA_Server *server,
     cb->default_interval = intervalMs;
     cb->registered = false;
     cb->client = client;
-    cb->discovery_server_url = discoveryServerUrl;
-
-
+    size_t len = strlen(discoveryServerUrl);
+    cb->discovery_server_url = (char*)UA_malloc(len+1);
+    if (!cb->discovery_server_url) {
+        UA_free(cb);
+        return UA_STATUSCODE_BADOUTOFMEMORY;
+    }
+    memcpy(cb->discovery_server_url, discoveryServerUrl, len+1);
 
     /* Add the callback */
     UA_StatusCode retval =