Merge branch 'fcarvajaldev-3.6.x-nxcomp-amend-compiler-warnings' into 3.6.x

5 files changed, 129 insertions, 61 deletions

diff --git a/nxcomp/Loop.cpp b/nxcomp/Loop.cpp
index 0f9fec6ef..a60bfc9ae 100644
--- a/nxcomp/Loop.cpp
+++ b/nxcomp/Loop.cpp
@@ -9393,12 +9393,7 @@ int ParseRemoteOptions(char *opts)
   int hasDelta  = 0;
   int hasStream = 0;
   int hasData   = 0;
-  int hasLimit  = 0;
-  int hasRender = 0;
-  int hasTaint  = 0;
   int hasType   = 0;
-  int hasStrict = 0;
-  int hasShseg  = 0;
 
   //
   // Get rid of the terminating space.
@@ -9623,7 +9618,6 @@ int ParseRemoteOptions(char *opts)
         }
       }
 
-      hasLimit = 1;
     }
     else if (strcasecmp(name, "render") == 0)
     {
@@ -9636,7 +9630,6 @@ int ParseRemoteOptions(char *opts)
         useRender = ValidateArg("remote", name, value);
       }
 
-      hasRender = 1;
     }
     else if (strcasecmp(name, "taint") == 0)
     {
@@ -9649,7 +9642,6 @@ int ParseRemoteOptions(char *opts)
         useTaint = ValidateArg("remote", name, value);
       }
 
-      hasTaint = 1;
     }
     else if (strcasecmp(name, "type") == 0)
     {
@@ -9682,7 +9674,6 @@ int ParseRemoteOptions(char *opts)
         useStrict = ValidateArg("remote", name, value);
       }
 
-      hasStrict = 1;
     }
     else if (strcasecmp(name, "shseg") == 0)
     {
@@ -9704,7 +9695,6 @@ int ParseRemoteOptions(char *opts)
         return -1;
       }
 
-      hasShseg = 1;
     }
     else if (strcasecmp(name, "delta") == 0)
     {
diff --git a/nxcomp/Pipe.cpp b/nxcomp/Pipe.cpp
index aacbbaeb3..15c7a31b3 100644
--- a/nxcomp/Pipe.cpp
+++ b/nxcomp/Pipe.cpp
@@ -237,8 +237,14 @@ FILE *Popen(char * const parameters[], const char *type)
 
       struct passwd *pwent = getpwuid(getuid());
       if (pwent) initgroups(pwent->pw_name,getgid());
-      setgid(getgid());
-      setuid(getuid());
+      if (setgid(getgid()) == -1)
+      {
+        _exit(127);
+      }
+      if (setuid(getuid()) == -1)
+      {
+        _exit(127);
+      }
 
       if (*type == 'r')
       {
diff --git a/nxcomp/Proxy.cpp b/nxcomp/Proxy.cpp
index ae115fede..7655ccb40 100644
--- a/nxcomp/Proxy.cpp
+++ b/nxcomp/Proxy.cpp
@@ -18,6 +18,7 @@
 #include <cstdio>
 #include <unistd.h>
 #include <cstdlib>
+#include <string.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #ifdef ANDROID
@@ -4194,6 +4195,12 @@ int Proxy::handleSaveStores()
 
   char *cacheToAdopt = NULL;
 
+  //
+  // Set to false the indicator for cumulative store
+  // size too small
+  //
+  bool isTooSmall = false;
+
   if (control -> PersistentCacheEnableSave)
   {
     #ifdef TEST
@@ -4201,7 +4208,7 @@ int Proxy::handleSaveStores()
             << logofs_flush;
     #endif
 
-    cacheToAdopt = handleSaveAllStores(control -> PersistentCachePath);
+    cacheToAdopt = handleSaveAllStores(control -> PersistentCachePath, isTooSmall);
   }
   #ifdef TEST
   else
@@ -4253,21 +4260,28 @@ int Proxy::handleSaveStores()
 
     return 1;
   }
-  #ifdef TEST
   else
   {
-    *logofs << "Proxy: No cache file produced from message stores.\n"
-            << logofs_flush;
-  }
-  #endif
+    #ifdef TEST
+      *logofs << "Proxy: No cache file produced from message stores.\n"
+              << logofs_flush;
+    #endif
 
-  //
-  // It can be that we didn't generate a new cache
-  // because store was too small or persistent cache
-  // was disabled. This is not an error.
-  //
+    //
+    // It can be that we didn't generate a new cache
+    // because store was too small or persistent cache
+    // was disabled. This is not an error.
+    //
 
-  return 0;
+    if (control -> PersistentCacheEnableSave && !isTooSmall)
+    {
+      return -1;
+    }
+    else
+    {
+      return 0;
+    }
+  }
 }
 
 int Proxy::handleLoadStores()
@@ -4875,8 +4889,10 @@ int Proxy::handleLoadVersion(const unsigned char *buffer, int &major,
   return 1;
 }
 
-char *Proxy::handleSaveAllStores(const char *savePath) const
+char *Proxy::handleSaveAllStores(const char *savePath, bool & isTooSmall) const
 {
+  isTooSmall = false;
+
   int cumulativeSize = MessageStore::getCumulativeTotalStorageSize();
 
   if (cumulativeSize < control -> PersistentCacheThreshold)
@@ -4888,6 +4904,13 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
             << ".\n" << logofs_flush;
     #endif
 
+    //
+    // Cumulative store size is smaller than threshold
+    // so the indicator is set to true
+    //
+
+    isTooSmall = true;
+
     return NULL;
   }
   else if (savePath == NULL)
@@ -4923,20 +4946,28 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
   md5_state_t *md5StateClient = NULL;
   md5_byte_t  *md5DigestClient = NULL;
 
-  char *tempName = NULL;
-
   char md5String[MD5_LENGTH * 2 + 2];
 
   char fullName[strlen(savePath) + MD5_LENGTH * 2 + 4];
 
-  if (control -> ProxyMode == proxy_client)
-  {
-    tempName = tempnam(savePath, "Z-C-");
-  }
-  else
-  {
-    tempName = tempnam(savePath, "Z-S-");
-  }
+  //
+  // Prepare the template for the temporary file
+  //
+
+  const char* const uniqueTemplate = "XXXXXX";
+  char tempName[strlen(savePath) + strlen("/") + 4 + strlen(uniqueTemplate) + 1];
+
+  snprintf(tempName, sizeof tempName, "%s/%s%s",
+               savePath,
+               control -> ProxyMode == proxy_client ?
+                   "Z-C-" :
+                   "Z-S-",
+               uniqueTemplate);
+
+  #ifdef TEST
+  *logofs << "Proxy: Generating temporary file with template '"
+          << tempName << "'.\n" << logofs_flush;
+  #endif
 
   //
   // Change the mask to make the file only
@@ -4946,35 +4977,68 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
 
   mode_t fileMode = umask(0077);
 
-  cachefs = new ofstream(tempName, ios::out | ios::binary);
-
-  umask(fileMode);
+  //
+  // Generate a unique temporary filename from tempName
+  // and then create and open the file
+  //
 
-  if (tempName == NULL || cachefs == NULL)
+  int fdTemp = mkstemp(tempName);
+  if (fdTemp == -1)
   {
     #ifdef PANIC
     *logofs << "Proxy: PANIC! Can't create temporary file in '"
-            << savePath << "'.\n" << logofs_flush;
+            << savePath << "'. Cause = " << strerror(errno) << ".\n" << logofs_flush;
     #endif
 
     cerr << "Error" << ": Can't create temporary file in '"
-         << savePath << "'.\n";
+         << savePath << "'. Cause = " << strerror(errno) << ".\n";
 
-    if (tempName != NULL)
-    {
-      free(tempName);
-    }
+    umask(fileMode);
 
-    if (cachefs != NULL)
-    {
-      delete cachefs;
-    }
+    EnableSignals();
+
+    return NULL;
+  }
+
+  #ifdef TEST
+  *logofs << "Proxy: Saving cache to file '"
+          << tempName << "'.\n" << logofs_flush;
+  #endif
+
+  //
+  // Create and open the output stream for the new temporary
+  // file
+  //
+
+  cachefs = new (std::nothrow) ofstream(tempName, ios::out | ios::binary);
+  if ((cachefs == NULL) || cachefs->fail())
+  {
+    #ifdef PANIC
+    *logofs << "Proxy: PANIC! Can't create stream for temporary file '"
+            << tempName << "'.\n" << logofs_flush;
+    #endif
+
+    cerr << "Error" << ": Can't create stream for temporary file '"
+         << tempName << "'.\n";
+
+    close(fdTemp);
+    unlink(tempName);
+
+    umask(fileMode);
 
     EnableSignals();
 
     return NULL;
   }
 
+  //
+  // Close the file descriptor returned by mkstemp
+  // and restore the old mask
+  //
+
+  close(fdTemp);
+  umask(fileMode);
+
   md5StateStream  = new md5_state_t();
   md5DigestStream = new md5_byte_t[MD5_LENGTH];
 
@@ -5007,8 +5071,6 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
     delete md5StateStream;
     delete [] md5DigestStream;
 
-    free(tempName);
-
     EnableSignals();
 
     return NULL;
@@ -5029,8 +5091,6 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
     delete md5StateStream;
     delete [] md5DigestStream;
 
-    free(tempName);
-
     EnableSignals();
 
     return NULL;
@@ -5086,7 +5146,7 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
 
   #endif
 
-  if (allSaved == 0)
+  if (allSaved == -1)
   {
     handleFailOnSave(tempName, "C");
 
@@ -5098,8 +5158,6 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
     delete md5StateClient;
     delete [] md5DigestClient;
 
-    free(tempName);
-
     EnableSignals();
 
     return NULL;
@@ -5139,8 +5197,6 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
     delete md5StateClient;
     delete [] md5DigestClient;
 
-    free(tempName);
-
     EnableSignals();
 
     return NULL;
@@ -5181,8 +5237,6 @@ char *Proxy::handleSaveAllStores(const char *savePath) const
   delete md5StateClient;
   delete [] md5DigestClient;
 
-  free(tempName);
-
   //
   // Restore the original handlers.
   //
diff --git a/nxcomp/Proxy.h b/nxcomp/Proxy.h
index 2f3be7596..c0fdfcce9 100644
--- a/nxcomp/Proxy.h
+++ b/nxcomp/Proxy.h
@@ -985,7 +985,7 @@ class Proxy
   int handleLoadStores();
   int handleSaveStores();
 
-  char *handleSaveAllStores(const char *savePath) const;
+  char *handleSaveAllStores(const char *savePath, bool & isTooSmall) const;
 
   virtual int handleSaveAllStores(ostream *cachefs, md5_state_t *md5StateStream,
                                       md5_state_t *md5StateClient) const = 0;
diff --git a/nxcomp/ServerChannel.cpp b/nxcomp/ServerChannel.cpp
index e0bd2132f..ee39903a7 100644
--- a/nxcomp/ServerChannel.cpp
+++ b/nxcomp/ServerChannel.cpp
@@ -4374,6 +4374,24 @@ int ServerChannel::handleWrite(const unsigned char *message, unsigned int length
       } // End of switch on opcode.
 
       //
+      // TODO: at the moment the variable hit was being set
+      // but no used, so to avoid the corresponding warning
+      // it has been added this block with a logging command.
+      // This code will be probably optimized away when none
+      // of the defines is set, but if there is no additional
+      // use for the hit variable in the future, then maybe
+      // it could be removed completely.
+      //
+
+      if (hit)
+      {
+        #if defined(TEST) || defined(OPCODES)
+        *logofs << "handleWrite: Cached flag enabled in handled request.\n"
+                << logofs_flush;
+        #endif
+      }
+
+      //
       // A packed image request can generate more than just
       // a single X_PutImage. Write buffer is handled inside
       // handleUnpack(). Cannot simply assume that the final