Return std::string instead of dynamic allocations

We mostly use classical C strings, but the memory management around them
can get confusing and error prone. Let's use std::string for the cases
where we need to return a newly allocated string.

11 files changed, 64 insertions, 81 deletions

diff --git a/win/rfb_win32/Clipboard.cxx b/win/rfb_win32/Clipboard.cxx
index 9a9dfb6a..d90d0b7a 100644
--- a/win/rfb_win32/Clipboard.cxx
+++ b/win/rfb_win32/Clipboard.cxx
@@ -86,11 +86,11 @@ Clipboard::processMessage(UINT msg, WPARAM wParam, LPARAM lParam) {
   return MsgWindow::processMessage(msg, wParam, lParam);
 };
 
-char*
+std::string
 Clipboard::getClipText() {
   HGLOBAL cliphandle;
   wchar_t* clipdata;
-  CharArray utf8;
+  std::string utf8;
 
   // Open the clipboard
   if (!OpenClipboard(getHandle()))
@@ -110,13 +110,13 @@ Clipboard::getClipText() {
   }
 
   // Convert it to UTF-8
-  utf8.replaceBuf(utf16ToUTF8(clipdata));
+  utf8 = utf16ToUTF8(clipdata);
 
   // Release the buffer and close the clipboard
   GlobalUnlock(cliphandle);
   CloseClipboard();
 
-  return convertLF(utf8.buf);
+  return convertLF(utf8.c_str());
 }
 
 void
@@ -130,20 +130,16 @@ Clipboard::setClipText(const char* text) {
       throw rdr::SystemException("unable to open Win32 clipboard", GetLastError());
 
     // - Convert the supplied clipboard text into UTF-16 format with CRLF
-    CharArray filtered(convertCRLF(text));
-    wchar_t* utf16;
-
-    utf16 = utf8ToUTF16(filtered.buf);
+    std::string filtered(convertCRLF(text));
+    std::wstring utf16(utf8ToUTF16(filtered.c_str()));
 
     // - Allocate global memory for the data
-    clip_handle = ::GlobalAlloc(GMEM_MOVEABLE, (wcslen(utf16) + 1) * 2);
+    clip_handle = ::GlobalAlloc(GMEM_MOVEABLE, (utf16.size() + 1) * 2);
 
     wchar_t* data = (wchar_t*) GlobalLock(clip_handle);
-    wcscpy(data, utf16);
+    wcscpy(data, utf16.c_str());
     GlobalUnlock(clip_handle);
 
-    strFree(utf16);
-
     // - Next, we must clear out any existing data
     if (!EmptyClipboard())
       throw rdr::SystemException("unable to empty Win32 clipboard", GetLastError());
diff --git a/win/rfb_win32/Clipboard.h b/win/rfb_win32/Clipboard.h
index 1dead82e..588f1086 100644
--- a/win/rfb_win32/Clipboard.h
+++ b/win/rfb_win32/Clipboard.h
@@ -50,7 +50,7 @@ namespace rfb {
       void setNotifier(Notifier* cbn) {notifier = cbn;}
 
       // - Get the clipboard contents
-      char* getClipText();
+      std::string getClipText();
 
       // - Set the clipboard contents
       void setClipText(const char* text);
diff --git a/win/rfb_win32/RegConfig.cxx b/win/rfb_win32/RegConfig.cxx
index 73a9e699..38ca52f9 100644
--- a/win/rfb_win32/RegConfig.cxx
+++ b/win/rfb_win32/RegConfig.cxx
@@ -26,7 +26,6 @@
 
 #include <rfb_win32/RegConfig.h>
 #include <rfb/LogWriter.h>
-#include <rfb/util.h>
 //#include <rdr/HexOutStream.h>
 
 using namespace rfb;
@@ -63,8 +62,8 @@ void RegConfig::loadRegistryConfig(RegKey& key) {
     while (1) {
       const char *name = key.getValueName(i++);
       if (!name) break;
-      CharArray value(key.getRepresentation(name));
-      if (!value.buf || !Configuration::setParam(name, value.buf))
+      std::string value = key.getRepresentation(name);
+      if (!Configuration::setParam(name, value.c_str()))
         vlog.info("unable to process %s", name);
     }
   } catch (rdr::SystemException& e) {
diff --git a/win/rfb_win32/Registry.cxx b/win/rfb_win32/Registry.cxx
index a994fe6f..dccaa727 100644
--- a/win/rfb_win32/Registry.cxx
+++ b/win/rfb_win32/Registry.cxx
@@ -164,18 +164,21 @@ void RegKey::setBool(const char* valname, bool value) const {
   setInt(valname, value ? 1 : 0);
 }
 
-char* RegKey::getString(const char* valname) const {return getRepresentation(valname);}
-char* RegKey::getString(const char* valname, const char* def) const {
+std::string RegKey::getString(const char* valname) const {
+  return getRepresentation(valname);
+}
+
+std::string RegKey::getString(const char* valname, const char* def) const {
   try {
     return getString(valname);
   } catch(rdr::Exception&) {
-    return strDup(def);
+    return def;
   }
 }
 
 std::vector<uint8_t> RegKey::getBinary(const char* valname) const {
-  CharArray hex(getRepresentation(valname));
-  return hexToBin(hex.buf, strlen(hex.buf));
+  std::string hex = getRepresentation(valname);
+  return hexToBin(hex.data(), hex.size());
 }
 std::vector<uint8_t> RegKey::getBinary(const char* valname, const uint8_t* def, size_t deflen) const {
   try {
@@ -188,8 +191,7 @@ std::vector<uint8_t> RegKey::getBinary(const char* valname, const uint8_t* def,
 }
 
 int RegKey::getInt(const char* valname) const {
-  CharArray tmp(getRepresentation(valname));
-  return atoi(tmp.buf);
+  return atoi(getRepresentation(valname).c_str());
 }
 int RegKey::getInt(const char* valname, int def) const {
   try {
@@ -206,17 +208,7 @@ bool RegKey::getBool(const char* valname, bool def) const {
   return getInt(valname, def ? 1 : 0) > 0;
 }
 
-static inline char* terminateData(char* data, int length)
-{
-  // We must terminate the string, just to be sure.  Stupid Win32...
-  int len = length/sizeof(char);
-  CharArray str(len+1);
-  memcpy(str.buf, data, length);
-  str.buf[len] = 0;
-  return str.takeBuf();
-}
-
-char* RegKey::getRepresentation(const char* valname) const {
+std::string RegKey::getRepresentation(const char* valname) const {
   DWORD type, length;
   LONG result = RegQueryValueEx(key, valname, 0, &type, 0, &length);
   if (result != ERROR_SUCCESS)
@@ -229,35 +221,34 @@ char* RegKey::getRepresentation(const char* valname) const {
   switch (type) {
   case REG_BINARY:
     {
-      CharArray hex(binToHex((const uint8_t*)data.buf, length));
-      return hex.takeBuf();
+      return binToHex((const uint8_t*)data.buf, length);
     }
   case REG_SZ:
     if (length) {
-      return terminateData(data.buf, length);
+      return std::string(data.buf, length);
     } else {
-      return strDup("");
+      return "";
     }
   case REG_DWORD:
     {
-      CharArray tmp(16);
-      sprintf(tmp.buf, "%lu", *((DWORD*)data.buf));
-      return tmp.takeBuf();
+      char tmp[16];
+      sprintf(tmp, "%lu", *((DWORD*)data.buf));
+      return tmp;
     }
   case REG_EXPAND_SZ:
     {
     if (length) {
-      CharArray str(terminateData(data.buf, length));
-      DWORD required = ExpandEnvironmentStrings(str.buf, 0, 0);
+      std::string str(data.buf, length);
+      DWORD required = ExpandEnvironmentStrings(str.c_str(), 0, 0);
       if (required==0)
         throw rdr::SystemException("ExpandEnvironmentStrings", GetLastError());
       CharArray result(required);
-      length = ExpandEnvironmentStrings(str.buf, result.buf, required);
+      length = ExpandEnvironmentStrings(str.c_str(), result.buf, required);
       if (required<length)
         throw rdr::Exception("unable to expand environment strings");
-      return result.takeBuf();
+      return result.buf;
     } else {
-      return strDup("");
+      return "";
     }
     }
   default:
@@ -267,7 +258,7 @@ char* RegKey::getRepresentation(const char* valname) const {
 
 bool RegKey::isValue(const char* valname) const {
   try {
-    CharArray tmp(getRepresentation(valname));
+    getRepresentation(valname);
     return true;
   } catch(rdr::Exception&) {
     return false;
diff --git a/win/rfb_win32/Registry.h b/win/rfb_win32/Registry.h
index 7292372f..a387472a 100644
--- a/win/rfb_win32/Registry.h
+++ b/win/rfb_win32/Registry.h
@@ -75,8 +75,8 @@ namespace rfb {
       void setInt(const char* valname, int i) const;
       void setBool(const char* valname, bool b) const;
 
-      char* getString(const char* valname) const;
-      char* getString(const char* valname, const char* def) const;
+      std::string getString(const char* valname) const;
+      std::string getString(const char* valname, const char* def) const;
 
       std::vector<uint8_t> getBinary(const char* valname) const;
       std::vector<uint8_t> getBinary(const char* valname, const uint8_t* def, size_t deflength) const;
@@ -87,7 +87,7 @@ namespace rfb {
       bool getBool(const char* valname) const;
       bool getBool(const char* valname, bool def) const;
 
-      char* getRepresentation(const char* valname) const;
+      std::string getRepresentation(const char* valname) const;
 
       bool isValue(const char* valname) const;
 
diff --git a/win/rfb_win32/SDisplay.cxx b/win/rfb_win32/SDisplay.cxx
index 110edcb0..811b1033 100644
--- a/win/rfb_win32/SDisplay.cxx
+++ b/win/rfb_win32/SDisplay.cxx
@@ -296,8 +296,7 @@ void SDisplay::restartCore() {
 
 
 void SDisplay::handleClipboardRequest() {
-  CharArray data(clipboard->getClipText());
-  server->sendClipboardData(data.buf);
+  server->sendClipboardData(clipboard->getClipText().c_str());
 }
 
 void SDisplay::handleClipboardAnnounce(bool available) {
diff --git a/win/rfb_win32/Win32Util.cxx b/win/rfb_win32/Win32Util.cxx
index 53443007..5f0bdbc7 100644
--- a/win/rfb_win32/Win32Util.cxx
+++ b/win/rfb_win32/Win32Util.cxx
@@ -67,9 +67,9 @@ const char* FileVersionInfo::getVerString(const char* name, DWORD langId) {
     langId = langId >> 8;
   }
 
-  CharArray langIdStr(binToHex(langIdBuf, sizeof(langId)));
-  CharArray infoName(strlen("StringFileInfo") + 4 + strlen(name) + strlen(langIdStr.buf));
-  sprintf(infoName.buf, "\\StringFileInfo\\%s\\%s", langIdStr.buf, name);
+  std::string langIdStr(binToHex(langIdBuf, sizeof(langId)));
+  CharArray infoName(strlen("StringFileInfo") + 4 + strlen(name) + strlen(langIdStr.c_str()));
+  sprintf(infoName.buf, "\\StringFileInfo\\%s\\%s", langIdStr.c_str(), name);
 
   // Locate the required version string within the version info
   char* buffer = 0;
diff --git a/win/vncconfig/Authentication.h b/win/vncconfig/Authentication.h
index 6789a4f2..cc162077 100644
--- a/win/vncconfig/Authentication.h
+++ b/win/vncconfig/Authentication.h
@@ -93,8 +93,8 @@ namespace rfb {
 
 #ifdef HAVE_GNUTLS
         if (isItemChecked(IDC_ENC_X509)) {
-          SSecurityTLS::X509_CertFile.setParam(regKey.getString("X509Cert"));
-          SSecurityTLS::X509_CertFile.setParam(regKey.getString("X509Key"));
+          SSecurityTLS::X509_CertFile.setParam(regKey.getString("X509Cert").c_str());
+          SSecurityTLS::X509_CertFile.setParam(regKey.getString("X509Key").c_str());
         }
 #endif
 
diff --git a/win/vncconfig/Connections.h b/win/vncconfig/Connections.h
index fbf46ec1..f9b66547 100644
--- a/win/vncconfig/Connections.h
+++ b/win/vncconfig/Connections.h
@@ -73,7 +73,7 @@ namespace rfb {
 
         try {
           network::TcpFilter::Pattern pat(network::TcpFilter::parsePattern(newPat.buf));
-          pattern.replaceBuf(CharArray(network::TcpFilter::patternToStr(pat)).takeBuf());
+          pattern.replaceBuf(strDup(network::TcpFilter::patternToStr(pat).c_str()));
         } catch(rdr::Exception& e) {
           MsgBox(NULL, e.str(), MB_ICONEXCLAMATION | MB_OK);
           return false;
@@ -101,7 +101,7 @@ namespace rfb {
           SendMessage(listBox, LB_DELETESTRING, 0, 0);
 
         CharArray tmp;
-        tmp.buf = hosts.getData();
+        tmp.buf = strDup(hosts.getValueStr().c_str());
         while (tmp.buf) {
           CharArray first;
           strSplit(tmp.buf, ',', &first.buf, &tmp.buf);
@@ -228,13 +228,13 @@ namespace rfb {
         regKey.setInt("PortNumber", isItemChecked(IDC_RFB_ENABLE) ? getItemInt(IDC_PORT) : 0);
         regKey.setInt("IdleTimeout", getItemInt(IDC_IDLE_TIMEOUT));
         regKey.setInt("LocalHost", isItemChecked(IDC_LOCALHOST));
-        regKey.setString("Hosts", CharArray(getHosts()).buf);
+        regKey.setString("Hosts", getHosts().c_str());
         return true;
       }
       bool isChanged() {
         try {
-          CharArray new_hosts(getHosts());
-          return (strcmp(new_hosts.buf, hosts) != 0) ||
+          std::string new_hosts = getHosts();
+          return (new_hosts != (const char*)hosts) ||
               (localHost != isItemChecked(IDC_LOCALHOST)) ||
               (port_number != getItemInt(IDC_PORT)) ||
               (rfb::Server::idleTimeout != getItemInt(IDC_IDLE_TIMEOUT));
@@ -242,21 +242,21 @@ namespace rfb {
           return false;
         }
       }
-      char* getHosts() {
+      std::string getHosts() {
         int bufLen = 1, i;
         HWND listBox = GetDlgItem(handle, IDC_HOSTS);
         for (i=0; i<SendMessage(listBox, LB_GETCOUNT, 0, 0); i++)
           bufLen+=SendMessage(listBox, LB_GETTEXTLEN, i, 0)+1;
-        CharArray hosts_str(bufLen);
-        hosts_str.buf[0] = 0;
-        char* outPos = hosts_str.buf;
+        std::vector<char> hosts_str(bufLen);
+        hosts_str[0] = 0;
+        char* outPos = hosts_str.data();
         for (i=0; i<SendMessage(listBox, LB_GETCOUNT, 0, 0); i++) {
           outPos += SendMessage(listBox, LB_GETTEXT, i, (LPARAM)outPos);
           outPos[0] = ',';
           outPos[1] = 0;
           outPos++;
         }
-        return strDup(hosts_str.buf);
+        return hosts_str.data();
       }
 
     protected:
diff --git a/win/vncconfig/Legacy.cxx b/win/vncconfig/Legacy.cxx
index e5433691..c9c5e697 100644
--- a/win/vncconfig/Legacy.cxx
+++ b/win/vncconfig/Legacy.cxx
@@ -62,15 +62,14 @@ void LegacyPage::LoadPrefs()
             regKey.setString("Log", logSetting);
           }
  
-          CharArray authHosts;
-          authHosts.buf = winvnc3.getString("AuthHosts", 0);
-          if (authHosts.buf) {
+          std::string authHosts = winvnc3.getString("AuthHosts", "");
+          if (!authHosts.empty()) {
             CharArray newHosts;
             newHosts.buf = strDup("");
 
             // Reformat AuthHosts to Hosts.  Wish I'd left the format the same. :( :( :(
             try {
-              CharArray tmp(authHosts.buf);
+              CharArray tmp(strDup(authHosts.c_str()));
               while (tmp.buf) {
 
                 // Split the AuthHosts string into patterns to match
diff --git a/win/winvnc/VNCServerWin32.cxx b/win/winvnc/VNCServerWin32.cxx
index 55efabe1..87258986 100644
--- a/win/winvnc/VNCServerWin32.cxx
+++ b/win/winvnc/VNCServerWin32.cxx
@@ -117,25 +117,24 @@ void VNCServerWin32::processAddressChange() {
     prefix = "VNC Server (Service):";
 
   // Fetch the list of addresses
-  std::list<char*> addrs;
+  std::list<std::string> addrs;
   if (rfbSock.isListening())
-    TcpListener::getMyAddresses(&addrs);
+    addrs = TcpListener::getMyAddresses();
   else
-    addrs.push_front(strDup("Not accepting connections"));
+    addrs.push_front("Not accepting connections");
 
   // Allocate space for the new tip
-  std::list<char*>::iterator i, next_i;
+  std::list<std::string>::iterator i, next_i;
   int length = strlen(prefix)+1;
   for (i=addrs.begin(); i!= addrs.end(); i++)
-    length += strlen(*i) + 1;
+    length += i->size() + 1;
 
   // Build the new tip
   CharArray toolTip(length);
   strcpy(toolTip.buf, prefix);
   for (i=addrs.begin(); i!= addrs.end(); i=next_i) {
     next_i = i; next_i ++;
-    CharArray addr(*i);    // Assumes ownership of string
-    strcat(toolTip.buf, addr.buf);
+    strcat(toolTip.buf, i->c_str());
     if (next_i != addrs.end())
       strcat(toolTip.buf, ",");
   }
@@ -230,11 +229,11 @@ bool VNCServerWin32::disconnectClients(const char* reason) {
 bool VNCServerWin32::addNewClient(const char* client) {
   TcpSocket* sock = 0;
   try {
-    CharArray hostname;
+    std::string hostname;
     int port;
-    getHostAndPort(client, &hostname.buf, &port, 5500);
+    getHostAndPort(client, &hostname, &port, 5500);
     vlog.error("port=%d", port);
-    sock = new TcpSocket(hostname.buf, port);
+    sock = new TcpSocket(hostname.c_str(), port);
     if (queueCommand(AddClient, sock, 0))
       return true;
     delete sock;