Merge branch 'uli42-pr/fix_memleaks' into 3.6.x

Attributes GH PR #575: https://github.com/ArcticaProject/nx-libs/pull/575

Fixes: ArcticaProject/nx-libs#569
Fixes: ArcticaProject/nx-libs#573

5 files changed, 57 insertions, 26 deletions

diff --git a/nxcomp/src/ChannelEndPoint.cpp b/nxcomp/src/ChannelEndPoint.cpp
index 4fdf0fad4..fb8549f6a 100644
--- a/nxcomp/src/ChannelEndPoint.cpp
+++ b/nxcomp/src/ChannelEndPoint.cpp
@@ -37,6 +37,8 @@
 
 #include "NXalert.h"
 
+#include "Misc.h"
+
 ChannelEndPoint::ChannelEndPoint(const char *spec)
   : defaultTCPPort_(0), defaultTCPInterface_(0),
     defaultUnixPath_(NULL), spec_(NULL) {
@@ -54,11 +56,14 @@ ChannelEndPoint::~ChannelEndPoint()
     if(S_ISSOCK(st.st_mode))
       unlink(unixPath);
   }
+  SAFE_FREE(unixPath);
+  SAFE_FREE(defaultUnixPath_);
+  SAFE_FREE(spec_);
 }
 
 void
 ChannelEndPoint::setSpec(const char *spec) {
-  if (spec_) free(spec_);
+  SAFE_FREE(spec_);
 
   if (spec && strlen(spec))
   {
@@ -90,10 +95,11 @@ void
 ChannelEndPoint::setSpec(const char *hostName, long port) {
   int length;
 
-  if (spec_) free(spec_);
   isUnix_ = false;
   isTCP_ = false;
 
+  SAFE_FREE(spec_);
+
   if (hostName && strlen(hostName) && port >= 1)
   {
     length = snprintf(NULL, 0, "tcp:%s:%ld", hostName, port);
@@ -137,9 +143,9 @@ ChannelEndPoint::getSpec(char **socketUri) const {
       *socketUri = strdup(newSocketUri);
   }
 
-  free(newSocketUri);
-  free(unixPath);
-  free(hostName);
+  SAFE_FREE(newSocketUri);
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
 
   if (NULL != *socketUri)
     return true;
@@ -160,7 +166,7 @@ ChannelEndPoint::setDefaultTCPInterface(int publicInterface) {
 
 void
 ChannelEndPoint::setDefaultUnixPath(char *path) {
-  if (defaultUnixPath_) free(defaultUnixPath_);
+  SAFE_FREE(defaultUnixPath_);
 
   if (path && strlen(path))
     defaultUnixPath_ = strdup(path);
@@ -193,7 +199,7 @@ ChannelEndPoint::getPort(long *port) const {
 bool
 ChannelEndPoint::getUnixPath(char **unixPath) const {
 
-  if (unixPath) *unixPath = 0;
+  if (unixPath) *unixPath = NULL;
 
   long p;
   char *path = NULL;
@@ -329,10 +335,10 @@ ChannelEndPoint &ChannelEndPoint::operator=(const ChannelEndPoint &other) {
   defaultTCPInterface_ = other.defaultTCPInterface_;
   old = defaultUnixPath_;
   defaultUnixPath_ = (other.defaultUnixPath_ ? strdup(other.defaultUnixPath_) : NULL);
-  free(old);
+  SAFE_FREE(old);
   old = spec_;
   spec_ = (other.spec_ ? strdup(other.spec_) : NULL);
-  free(old);
+  SAFE_FREE(old);
   isUnix_ = getUnixPath();
   isTCP_ = getTCPHostAndPort();
   return *this;
@@ -344,7 +350,7 @@ std::ostream& operator<<(std::ostream& os, const ChannelEndPoint& endPoint) {
     if (endPoint.getSpec(&endPointSpec))
     {
       os << endPointSpec;
-      free(endPointSpec);
+      SAFE_FREE(endPointSpec);
     }
     else
       os << "(invalid)";
diff --git a/nxcomp/src/Log.h b/nxcomp/src/Log.h
index aed929b31..3e355a951 100644
--- a/nxcomp/src/Log.h
+++ b/nxcomp/src/Log.h
@@ -168,7 +168,13 @@ class NXLog
         delete pdt->thread_name;
 
         while (!pdt->buffer.empty()) {
+            /*
+             * get the stringstream object created in new_stack_entry()
+             * from the stack and delete it after pop()
+             */
+            std::stringstream* tmp = pdt->buffer.top();
             (void) pdt->buffer.pop ();
+            delete tmp;
         }
 
         delete pdt;
@@ -240,7 +246,12 @@ class NXLog
         pthread_sigmask(SIG_BLOCK, &tmp_signal_mask, &orig_signal_mask);
 
         if (!pdt->buffer.empty ()) {
-            const std::string str = pdt->buffer.top()->str();
+            /*
+             * get the stringstream object created in new_stack_entry()
+             * from the stack and delete it after pop()
+             */
+            std::stringstream *tmp = pdt->buffer.top();
+            const std::string str = tmp->str();
 
             if (!str.empty())
             {
@@ -251,6 +262,9 @@ class NXLog
 
             /* Remove from stack. */
             pdt->buffer.pop();
+
+            /* free memory */
+            delete tmp;
         }
 
         /* Restore old signal mask. */
diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp
index b51d7e7e3..baad17699 100644
--- a/nxcomp/src/Loop.cpp
+++ b/nxcomp/src/Loop.cpp
@@ -3187,8 +3187,7 @@ int SetupProxyConnection()
     nxinfo << "Loop: listenSocket is "<< ( listenSocket.enabled() ? "enabled" : "disabled") << ". "
            << "The socket URI is '"<< ( socketUri != NULL ? socketUri : "<unset>") << "'.\n" << std::flush;
 
-    free(socketUri);
-    socketUri = NULL;
+    SAFE_FREE(socketUri);
 
     if (WE_INITIATE_CONNECTION)
     {
@@ -3196,7 +3195,7 @@ int SetupProxyConnection()
       {
         nxinfo << "Loop: Going to connect to '" << socketUri
                << "'.\n" << std::flush;
-        free(socketUri);
+        SAFE_FREE(socketUri);
 
         proxyFD = ConnectToRemote(connectSocket);
 
@@ -3219,7 +3218,7 @@ int SetupProxyConnection()
       {
         nxinfo << "Loop: Going to wait for connection at '"
                << socketUri << "'.\n" << std::flush;
-        free(socketUri);
+        SAFE_FREE(socketUri);
 
         proxyFD = WaitForRemote(listenSocket);
 
@@ -4278,15 +4277,18 @@ int ListenConnectionTCP(const char *host, long port, const char *label)
 
 int ListenConnection(ChannelEndPoint &endpoint, const char *label)
 {
-  char *unixPath, *host;
+  char *unixPath = NULL, *host = NULL;
   long port;
+  int result = -1;
   if (endpoint.getUnixPath(&unixPath)) {
-    return ListenConnectionUnix(unixPath, label);
+    result = ListenConnectionUnix(unixPath, label);
   }
   else if (endpoint.getTCPHostAndPort(&host, &port)) {
-    return ListenConnectionTCP(host, port, label);
+    result = ListenConnectionTCP(host, port, label);
   }
-  return -1;
+  SAFE_FREE(unixPath);
+  SAFE_FREE(host);
+  return result;
 }
 
 static int AcceptConnection(int fd, int domain, const char *label)
@@ -6217,7 +6219,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
   cerr << "Info" << ": Waiting for connection from "
        << hostLabel << " on socket '" << socketUri
        << "'.\n";
-  free(socketUri);
+  SAFE_FREE(socketUri);
 
   //
   // How many times to loop waiting for connections
@@ -6306,7 +6308,7 @@ int WaitForRemote(ChannelEndPoint &socketAddress)
 
         cerr << "Info" << ": Accepted connection from this host on Unix file socket '"
              << unixPath << "'.\n";
-        free(unixPath);
+        SAFE_FREE(unixPath);
 
         break;
       }
@@ -6739,10 +6741,16 @@ int ConnectToRemote(ChannelEndPoint &socketAddress)
     }
   }
 
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
+
   return pFD;
 
 ConnectToRemoteError:
 
+  SAFE_FREE(unixPath);
+  SAFE_FREE(hostName);
+
   if (pFD != -1)
   {
     close(pFD);
@@ -7938,7 +7946,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         cerr << "Error" << ": Refusing 'listen' parameter with 'connect' being '"
              << socketUri << "'.\n";
 
-        free(socketUri);
+        SAFE_FREE(socketUri);
         return -1;
       }
 
@@ -7966,7 +7974,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         cerr << "Error" << ": Refusing 'accept' parameter with 'connect' being '"
              << socketUri << "'.\n";
 
-	free(socketUri);
+        SAFE_FREE(socketUri);
         return -1;
       }
 
diff --git a/nxcomp/src/Misc.h b/nxcomp/src/Misc.h
index 997630137..7808c34c2 100644
--- a/nxcomp/src/Misc.h
+++ b/nxcomp/src/Misc.h
@@ -54,6 +54,9 @@ using namespace std;
 #define EGET()   (errno)
 #define ESTR()   strerror(errno)
 
+// a free() macro that clears the ptr after free
+#define SAFE_FREE(ptr) do { free(ptr); ptr = NULL; } while (0)
+
 //
 // TCP port offset applied to NX port specification.
 //
diff --git a/nxcomp/src/Pipe.cpp b/nxcomp/src/Pipe.cpp
index 4fa149412..228c556ae 100644
--- a/nxcomp/src/Pipe.cpp
+++ b/nxcomp/src/Pipe.cpp
@@ -203,7 +203,7 @@ FILE *Popen(char * const parameters[], const char *type)
 
   if (pipe(pdes) < 0)
   {
-    free(cur);
+    SAFE_FREE(cur);
 
     return NULL;
   }
@@ -237,7 +237,7 @@ FILE *Popen(char * const parameters[], const char *type)
       close(pdes[0]);
       close(pdes[1]);
 
-      free(cur);
+      SAFE_FREE(cur);
 
       return NULL;
     }
@@ -420,7 +420,7 @@ int Pclose(FILE *iop)
     last -> next = cur -> next;
   }
 
-  free(cur);
+  SAFE_FREE(cur);
 
   //
   // Child has finished and we called the