nxcomp: drop strncpy in favour of snprintf

with very few exceptions which require careful thinking ;-)

3 files changed, 33 insertions, 44 deletions

diff --git a/nxcomp/src/Auth.cpp b/nxcomp/src/Auth.cpp
index d398f5f85..87955f6ab 100644
--- a/nxcomp/src/Auth.cpp
+++ b/nxcomp/src/Auth.cpp
@@ -212,16 +212,14 @@ int Auth::getCookie()
 
   if (environment != NULL && *environment != '\0')
   {
-    strncpy(file_, environment, DEFAULT_STRING_LIMIT - 1);
+    snprintf(file_, DEFAULT_STRING_LIMIT, "%s", environment);
   }
   else
   {
-    snprintf(file_, DEFAULT_STRING_LIMIT - 1, "%s/.Xauthority",
+    snprintf(file_, DEFAULT_STRING_LIMIT, "%s/.Xauthority",
                  control -> HomePath);
   }
 
-  *(file_ + DEFAULT_STRING_LIMIT - 1) = '\0';
-
   #ifdef TEST
   *logofs << "Auth: Using X authorization file '" << file_
           << "'.\n" << logofs_flush;
@@ -242,18 +240,14 @@ int Auth::getCookie()
 
   #if defined(__CYGWIN32__)
 
-  snprintf(command, DEFAULT_STRING_LIMIT - 1,
+  snprintf(command, DEFAULT_STRING_LIMIT,
                "%s/bin/nxauth", control -> SystemPath);
 
-  *(command + DEFAULT_STRING_LIMIT - 1) = '\0';
-
   #elif defined(__APPLE__)
 
-  snprintf(command, DEFAULT_STRING_LIMIT - 1,
+  snprintf(command, DEFAULT_STRING_LIMIT,
                "%s/nxauth", control -> SystemPath);
 
-  *(command + DEFAULT_STRING_LIMIT - 1) = '\0';
-
   #else
 
   strcpy(command, "xauth");
diff --git a/nxcomp/src/Children.cpp b/nxcomp/src/Children.cpp
index 9486f189a..e586292da 100644
--- a/nxcomp/src/Children.cpp
+++ b/nxcomp/src/Children.cpp
@@ -275,12 +275,14 @@ int NXTransDialog(const char *caption, const char *message,
 
       #ifdef __APPLE__
 
+      // FIXME: missing length limitation!
       strcat(newPath, "/Applications/NX Client for OSX.app/Contents/MacOS:");
 
       #endif
 
       #ifdef __CYGWIN32__
 
+      // FIXME: missing length limitation!
       strcat(newPath, ".:");
 
       #endif
@@ -289,9 +291,8 @@ int NXTransDialog(const char *caption, const char *message,
 
       char *oldPath = getenv("PATH");
 
-      strncpy(newPath + newLength, oldPath, DEFAULT_STRING_LIMIT - newLength - 1);
-
-      newPath[DEFAULT_STRING_LIMIT - 1] = '\0';
+      // FIXME: check if strncat would be better here
+      snprintf(newPath + newLength, DEFAULT_STRING_LIMIT - newLength, "%s", oldPath);
 
       #ifdef WARNING
       *logofs << "NXTransDialog: WARNING! Trying with path '"
@@ -427,17 +428,13 @@ int NXTransClient(const char* display)
 
   #ifdef __sun
 
-  snprintf(newDisplay, DISPLAY_LENGTH_LIMIT - 1, "DISPLAY=%s", display);
-
-  newDisplay[DISPLAY_LENGTH_LIMIT - 1] = '\0';
+  snprintf(newDisplay, DISPLAY_LENGTH_LIMIT, "DISPLAY=%s", display);
 
   putenv(newDisplay);
 
   #else
 
-  strncpy(newDisplay, display, DISPLAY_LENGTH_LIMIT - 1);
-
-  newDisplay[DISPLAY_LENGTH_LIMIT - 1] = '\0';
+  snprintf(newDisplay, DISPLAY_LENGTH_LIMIT, "%s", display);
 
   setenv("DISPLAY", newDisplay, 1);
 
@@ -467,6 +464,7 @@ int NXTransClient(const char* display)
     if (i == 0)
     {
 
+      // FIXME: code dpulication: this whole block is duplicated in NXTransDialog
       strcpy(command, "nxclient");
 
       char newPath[DEFAULT_STRING_LIMIT];
@@ -489,7 +487,8 @@ int NXTransClient(const char* display)
 
       char *oldPath = getenv("PATH");
 
-      strncpy(newPath + newLength, oldPath, DEFAULT_STRING_LIMIT - newLength - 1);
+      // FIXME: check if strncat would be better here
+      snprintf(newPath + newLength, DEFAULT_STRING_LIMIT - newLength, "%s", oldPath);
 
       newPath[DEFAULT_STRING_LIMIT - 1] = '\0';
 
diff --git a/nxcomp/src/Loop.cpp b/nxcomp/src/Loop.cpp
index b0b778a14..6b903789b 100644
--- a/nxcomp/src/Loop.cpp
+++ b/nxcomp/src/Loop.cpp
@@ -4044,7 +4044,7 @@ void SetupDisplaySocket(int &addr_family, sockaddr *&addr,
 
     if (useLaunchdSocket == 1)
     {
-      strncpy(unixSocketName, displayHost, DEFAULT_STRING_LENGTH - 1);
+      snprintf(unixSocketName, DEFAULT_STRING_LENGTH, "%s", displayHost);
     }
 
     #endif
@@ -7923,11 +7923,11 @@ int ParseEnvironmentOptions(const char *env, int force)
 
     if (strcasecmp(name, "options") == 0)
     {
-      strncpy(fileOptions, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(fileOptions, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "display") == 0)
     {
-      strncpy(displayHost, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(displayHost, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "link") == 0)
     {
@@ -7983,7 +7983,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         }
         else
         {
-          strncpy(sessionType, value, DEFAULT_STRING_LENGTH - 1);
+          snprintf(sessionType, DEFAULT_STRING_LENGTH, "%s", value);
         }
       }
     }
@@ -8036,7 +8036,7 @@ int ParseEnvironmentOptions(const char *env, int force)
         return -1;
       }
 
-      strncpy(acceptHost, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(acceptHost, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "connect") == 0)
     {
@@ -8074,7 +8074,7 @@ int ParseEnvironmentOptions(const char *env, int force)
     }
     else if (strcasecmp(name, "session") == 0)
     {
-      strncpy(sessionFileName, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(sessionFileName, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "errors") == 0)
     {
@@ -8085,27 +8085,27 @@ int ParseEnvironmentOptions(const char *env, int force)
       // the same name.
       //
 
-      strncpy(errorsFileName, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(errorsFileName, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "root") == 0)
     {
-      strncpy(rootDir, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(rootDir, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "id") == 0)
     {
-      strncpy(sessionId, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(sessionId, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "stats") == 0)
     {
       control -> EnableStatistics = 1;
 
-      strncpy(statsFileName, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(statsFileName, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "cookie") == 0)
     {
       LowercaseArg("local", name, value);
 
-      strncpy(authCookie, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(authCookie, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "nodelay") == 0)
     {
@@ -8334,7 +8334,7 @@ int ParseEnvironmentOptions(const char *env, int force)
     }
     else if (strcasecmp(name, "font") == 0)
     {
-      strncpy(fontPort, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(fontPort, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "slave") == 0)
     {
@@ -8439,7 +8439,7 @@ int ParseEnvironmentOptions(const char *env, int force)
     }
     else if (strcasecmp(name, "product") == 0)
     {
-      strncpy(productName, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(productName, DEFAULT_STRING_LENGTH, "%s", value);
     }
     else if (strcasecmp(name, "rootless") == 0 ||
                  strcasecmp(name, "geometry") == 0 ||
@@ -8529,7 +8529,7 @@ int ParseEnvironmentOptions(const char *env, int force)
 
     if (*optionsFileName == '\0')
     {
-      strncpy(optionsFileName, value, DEFAULT_STRING_LENGTH - 1);
+      snprintf(optionsFileName, DEFAULT_STRING_LENGTH, "%s", value);
 
       nxinfo << "Loop: Assuming name of options file '"
              << optionsFileName << "'.\n"
@@ -9249,7 +9249,7 @@ int ParseRemoteOptions(char *opts)
         }
         else
         {
-          strncpy(sessionType, value, DEFAULT_STRING_LENGTH - 1);
+          snprintf(sessionType, DEFAULT_STRING_LENGTH, "%s", value);
         }
       }
 
@@ -12719,6 +12719,7 @@ int ParseHostOption(const char *opt, char *host, long &port)
 
   char newHost[DEFAULT_STRING_LENGTH] = { 0 };
 
+  // opt cannot be longer than DEFAULT_STRING_LENGTH, this is checked above
   strncpy(newHost, opt, strlen(opt) - strlen(separator));
 
   *(newHost + strlen(opt) - strlen(separator)) = '\0';
@@ -13491,10 +13492,8 @@ int ParseArg(const char *type, const char *name, const char *value)
 
   char *string = new char[strlen(value)];
 
-  strncpy(string, value, strlen(value) - 1);
-
-  *(string + (strlen(value) - 1)) = '\0';
-
+  // copy value but cut off the last character
+  snprintf(string, strlen(value), "%s", value);
 
   nxinfo << "Loop: Parsing integer option '" << name
          << "' from string '" << string << "' with base set to ";
@@ -13512,18 +13511,15 @@ int ParseArg(const char *type, const char *name, const char *value)
 
   nxinfo_append << ".\n" << std::flush;
 
-
   double result = atof(string) * base;
 
+  delete [] string;
+
   if (result < 0 || result > (((unsigned) -1) >> 1))
   {
-    delete [] string;
-
     return -1;
   }
 
-  delete [] string;
-
   nxinfo << "Loop: Integer option parsed to '"
          << (int) result << "'.\n" << std::flush;