Fix memory leak, add volatile, reduce duplication

- config.c: free partial strdup on configAppend failure
- common.c: consolidate loadTrustStore error paths with goto
- server_lds.c, server_register.c: make running volatile, remove
  non-async-signal-safe call from signal handler
- server_register.c: extract LdsClientParams + makeLdsClientConfig
  to deduplicate the register/deregister client config setup

4 files changed, 91 insertions, 62 deletions

diff --git a/src/common.c b/src/common.c
index 8d7d651..39a2a68 100644
--- a/src/common.c
+++ b/src/common.c
@@ -74,6 +74,7 @@ loadTrustStore (const char *dirPath, char ***outPaths, size_t *outSize)
       return -1;
     }
 
+  int rc = -1;
   size_t capacity = 8;
   size_t count = 0;
   char **paths = malloc (capacity * sizeof (char *));
@@ -81,8 +82,7 @@ loadTrustStore (const char *dirPath, char ***outPaths, size_t *outSize)
     {
       UA_LOG_ERROR (UA_Log_Stdout, UA_LOGCATEGORY_USERLAND,
                     "loadTrustStore: out of memory");
-      closedir (dir);
-      return -1;
+      goto cleanup;
     }
 
   struct dirent *entry;
@@ -94,22 +94,6 @@ loadTrustStore (const char *dirPath, char ***outPaths, size_t *outSize)
       if (nameLen < 5 || strcmp (name + nameLen - 4, ".der") != 0)
         continue;
 
-      /* Build full path: dirPath/name */
-      size_t dirLen = strlen (dirPath);
-      size_t fullLen = dirLen + 1 + nameLen + 1;
-      char *full = malloc (fullLen);
-      if (!full)
-        {
-          UA_LOG_ERROR (UA_Log_Stdout, UA_LOGCATEGORY_USERLAND,
-                        "loadTrustStore: out of memory");
-          for (size_t i = 0; i < count; i++)
-            free (paths[i]);
-          free (paths);
-          closedir (dir);
-          return -1;
-        }
-      snprintf (full, fullLen, "%s/%s", dirPath, name);
-
       if (count == capacity)
         {
           capacity *= 2;
@@ -118,30 +102,41 @@ loadTrustStore (const char *dirPath, char ***outPaths, size_t *outSize)
             {
               UA_LOG_ERROR (UA_Log_Stdout, UA_LOGCATEGORY_USERLAND,
                             "loadTrustStore: out of memory");
-              free (full);
-              for (size_t i = 0; i < count; i++)
-                free (paths[i]);
-              free (paths);
-              closedir (dir);
-              return -1;
+              goto cleanup;
             }
           paths = tmp;
         }
 
+      /* Build full path: dirPath/name */
+      size_t dirLen = strlen (dirPath);
+      size_t fullLen = dirLen + 1 + nameLen + 1;
+      char *full = malloc (fullLen);
+      if (!full)
+        {
+          UA_LOG_ERROR (UA_Log_Stdout, UA_LOGCATEGORY_USERLAND,
+                        "loadTrustStore: out of memory");
+          goto cleanup;
+        }
+      snprintf (full, fullLen, "%s/%s", dirPath, name);
+
       paths[count++] = full;
     }
 
-  closedir (dir);
-
-  if (count == 0)
+  rc = 0;
+  if (count > 0)
     {
-      free (paths);
-      return 0;
+      *outPaths = paths;
+      *outSize = count;
+      paths = NULL;
+      count = 0;
     }
 
-  *outPaths = paths;
-  *outSize = count;
-  return 0;
+cleanup:
+  for (size_t i = 0; i < count; i++)
+    free (paths[i]);
+  free (paths);
+  closedir (dir);
+  return rc;
 }
 
 void
diff --git a/src/config.c b/src/config.c
index 2165821..258d167 100644
--- a/src/config.c
+++ b/src/config.c
@@ -68,6 +68,8 @@ configAppend (Config *cfg, const char *key, const char *value)
   cfg->entries[cfg->count].value = strdup (value);
   if (!cfg->entries[cfg->count].key || !cfg->entries[cfg->count].value)
     {
+      free (cfg->entries[cfg->count].key);
+      free (cfg->entries[cfg->count].value);
       UA_LOG_ERROR (UA_Log_Stdout, UA_LOGCATEGORY_USERLAND,
                     "Config: out of memory");
       return -1;
diff --git a/src/server_lds.c b/src/server_lds.c
index 0a1bf89..f1efa99 100644
--- a/src/server_lds.c
+++ b/src/server_lds.c
@@ -20,12 +20,11 @@
 #include <signal.h>
 #include <stdlib.h>
 
-UA_Boolean running = true;
+volatile UA_Boolean running = true;
 
 static void
 stopHandler (int sig)
 {
-  UA_LOG_INFO (UA_Log_Stdout, UA_LOGCATEGORY_SERVER, "received ctrl-c");
   running = false;
 }
 
diff --git a/src/server_register.c b/src/server_register.c
index 80bed20..ec5045f 100644
--- a/src/server_register.c
+++ b/src/server_register.c
@@ -22,16 +22,58 @@
 #include <string.h>
 #include <time.h>
 
-UA_Boolean running = true;
+volatile UA_Boolean running = true;
 
 static void
 stopHandler (int sign)
 {
-  UA_LOG_INFO (UA_Log_Stdout, UA_LOGCATEGORY_SERVER, "received ctrl-c");
   running = false;
 }
 
 /* ========================================================================
+ * LDS Client Config Helper
+ * ======================================================================== */
+
+/**
+ * Parameters for building a client config that connects to the LDS.
+ * Gathered once in main, then reused for every register/deregister call.
+ */
+typedef struct
+{
+  const char *appUri;
+  const char *certPath;
+  const char *keyPath;
+  char **trustPaths;
+  size_t trustSize;
+  UA_MessageSecurityMode securityMode;
+  const char *securityPolicyUri;
+  int logLevel;
+  const char *username;
+  const char *password;
+} LdsClientParams;
+
+/**
+ * Builds a fresh UA_ClientConfig from LdsClientParams.
+ *
+ * UA_Server_registerDiscovery / deregisterDiscovery consume (clear) the
+ * client config, so a new one must be created before every call.
+ */
+static UA_StatusCode
+makeLdsClientConfig (UA_ClientConfig *cc, const LdsClientParams *p)
+{
+  memset (cc, 0, sizeof (UA_ClientConfig));
+  UA_StatusCode rv = createSecureClientConfig (
+      cc, p->appUri, p->certPath, p->keyPath, p->trustPaths, p->trustSize,
+      p->securityMode, p->securityPolicyUri);
+  if (rv != UA_STATUSCODE_GOOD)
+    return rv;
+  cc->logging->context = (void *)(uintptr_t)p->logLevel;
+  if (p->username)
+    UA_ClientConfig_setAuthenticationUsername (cc, p->username, p->password);
+  return UA_STATUSCODE_GOOD;
+}
+
+/* ========================================================================
  * Main
  * ======================================================================== */
 
@@ -196,26 +238,32 @@ main (int argc, char **argv)
   serverConfig->applicationDescription.applicationType
       = UA_APPLICATIONTYPE_SERVER;
 
+  LdsClientParams ldsParams = {
+    .appUri = clientAppUri,
+    .certPath = clientCertPath,
+    .keyPath = clientKeyPath,
+    .trustPaths = clientTrustPaths,
+    .trustSize = clientTrustSize,
+    .securityMode = securityMode,
+    .securityPolicyUri = securityPolicyUri,
+    .logLevel = logLevel,
+    .username = clientUsername,
+    .password = clientPassword,
+  };
+
   /* Use run_startup + manual event loop (instead of UA_Server_run) so we
      can periodically re-register with the LDS between iterations. */
   UA_Server_run_startup (server);
 
   /* UA_Server_registerDiscovery consumes (clears) the client config,
-     so a fresh zero-initialized config is needed for every call. */
+     so makeLdsClientConfig builds a fresh one for every call. */
   UA_ClientConfig clientConfig;
-  memset (&clientConfig, 0, sizeof (UA_ClientConfig));
-  retval = createSecureClientConfig (
-      &clientConfig, clientAppUri, clientCertPath, clientKeyPath,
-      clientTrustPaths, clientTrustSize, securityMode, securityPolicyUri);
+  retval = makeLdsClientConfig (&clientConfig, &ldsParams);
   if (retval != UA_STATUSCODE_GOOD)
     {
       UA_Server_run_shutdown (server);
       goto cleanup;
     }
-  clientConfig.logging->context = (void *)(uintptr_t)logLevel;
-  if (clientUsername)
-    UA_ClientConfig_setAuthenticationUsername (&clientConfig, clientUsername,
-                                               clientPassword);
 
   UA_String discoveryUrl = UA_STRING_ALLOC (discoveryEndpoint);
   retval = UA_Server_registerDiscovery (server, &clientConfig, discoveryUrl,
@@ -237,17 +285,9 @@ main (int argc, char **argv)
       time_t now = time (NULL);
       if (now - lastRegister >= registerInterval)
         {
-          memset (&clientConfig, 0, sizeof (UA_ClientConfig));
-          retval = createSecureClientConfig (&clientConfig, clientAppUri,
-                                             clientCertPath, clientKeyPath,
-                                             clientTrustPaths, clientTrustSize,
-                                             securityMode, securityPolicyUri);
+          retval = makeLdsClientConfig (&clientConfig, &ldsParams);
           if (retval == UA_STATUSCODE_GOOD)
             {
-              clientConfig.logging->context = (void *)(uintptr_t)logLevel;
-              if (clientUsername)
-                UA_ClientConfig_setAuthenticationUsername (
-                    &clientConfig, clientUsername, clientPassword);
               UA_String reregUrl = UA_STRING_ALLOC (discoveryEndpoint);
               retval = UA_Server_registerDiscovery (server, &clientConfig,
                                                     reregUrl, UA_STRING_NULL);
@@ -263,16 +303,9 @@ main (int argc, char **argv)
 
   /* Deregister from the LDS before shutting down so the LDS removes
      our entry immediately rather than waiting for the cleanup timeout. */
-  memset (&clientConfig, 0, sizeof (UA_ClientConfig));
-  retval = createSecureClientConfig (
-      &clientConfig, clientAppUri, clientCertPath, clientKeyPath,
-      clientTrustPaths, clientTrustSize, securityMode, securityPolicyUri);
+  retval = makeLdsClientConfig (&clientConfig, &ldsParams);
   if (retval == UA_STATUSCODE_GOOD)
     {
-      clientConfig.logging->context = (void *)(uintptr_t)logLevel;
-      if (clientUsername)
-        UA_ClientConfig_setAuthenticationUsername (
-            &clientConfig, clientUsername, clientPassword);
       UA_String deregUrl = UA_STRING_ALLOC (discoveryEndpoint);
       retval = UA_Server_deregisterDiscovery (server, &clientConfig, deregUrl);
       UA_String_clear (&deregUrl);