Merge pull request #2 from MaxDZ8/bugfix-40-memory-leaks

Fix some more memory leaks by using string objects rather than C arrays where appropriate.
No more leaks on Windows, thanks to these changes contributed by Massimo Del Zotto.

src/gflags.cc

@@ -1463,11 +1463,16 @@ void GetAllFlags(vector<CommandLineFlagInfo>* OUTPUT) {
 
 // These values are not protected by a Mutex because they are normally
 // set only once during program startup.
-static const char* argv0 = "UNKNOWN";      // just the program name
-static const char* cmdline = "";           // the entire command-line
+//
+// For some reason those were raw const char* allocated in SetArgv and nobody
+// cared about releasing them. Oddly enough, SetArgv itself populates argvs
+// which is a vector of std::string so it appears there's no real problem in
+// using objects here.
+static string argv0("UNKNOWN");      // just the program name
+static string cmdline("");           // the entire command-line
 static vector<string> argvs;
-static uint32 argv_sum = 0;
-static const char* program_usage = NULL;
+static uint32 argv_sum = 0; // ?what is this for?
+static string program_usage;
 
 void SetArgv(int argc, const char** argv) {
   static bool called_set_argv = false;
@@ -1477,8 +1482,7 @@ void SetArgv(int argc, const char** argv) {
   called_set_argv = true;
 
   assert(argc > 0);            // every program has at least a progname
-  argv0 = strdup(argv[0]);     // small memory leak, but fn only called once
-  assert(argv0);
+  argv0 = argv[0];
 
   string cmdline_string;       // easier than doing strcats
   for (int i = 0; i < argc; i++) {
@@ -1488,38 +1492,37 @@ void SetArgv(int argc, const char** argv) {
     cmdline_string += argv[i];
     argvs.push_back(argv[i]);
   }
-  cmdline = strdup(cmdline_string.c_str());  // another small memory leak
-  assert(cmdline);
+  cmdline = cmdline_string.c_str();
 
   // Compute a simple sum of all the chars in argv
-  for (const char* c = cmdline; *c; c++)
+  for (string::const_iterator c = cmdline.cbegin(); c != cmdline.cend(); ++c)
     argv_sum += *c;
 }
 
 const vector<string>& GetArgvs() { return argvs; }
-const char* GetArgv()            { return cmdline; }
-const char* GetArgv0()           { return argv0; }
+const char* GetArgv()            { return cmdline.c_str(); }
+const char* GetArgv0()           { return argv0.c_str(); }
 uint32 GetArgvSum()              { return argv_sum; }
 const char* ProgramInvocationName() {             // like the GNU libc fn
   return GetArgv0();
 }
 const char* ProgramInvocationShortName() {        // like the GNU libc fn
-  const char* slash = strrchr(argv0, '/');
+  const char* slash = strrchr(argv0.c_str(), '/');
 #ifdef OS_WINDOWS
   if (!slash)  slash = strrchr(argv0, '\\');
 #endif
-  return slash ? slash + 1 : argv0;
+  return slash ? slash + 1 : argv0.c_str();
 }
 
 void SetUsageMessage(const string& usage) {
-  if (program_usage != NULL)
+  if (program_usage.length())
     ReportError(DIE, "ERROR: SetUsageMessage() called twice\n");
-  program_usage = strdup(usage.c_str());      // small memory leak
+  program_usage = usage;      // small memory leak
 }
 
 const char* ProgramUsage() {
-  if (program_usage) {
-    return program_usage;
+  if (program_usage.length()) {
+    return program_usage.c_str();
   }
   return "Warning: SetUsageMessage() never called";
 }
@@ -1529,16 +1532,16 @@ const char* ProgramUsage() {
 // VersionString()
 // --------------------------------------------------------------------
 
-static const char* version_string = NULL;
+static string version_string;
 
 void SetVersionString(const string& version) {
-  if (version_string != NULL)
+  if (version_string.empty() == false)
     ReportError(DIE, "ERROR: SetVersionString() called twice\n");
-  version_string = strdup(version.c_str());   // small memory leak
+  version_string = version;
 }
 
 const char* VersionString() {
-  return version_string ? version_string : "";
+  return version_string.c_str();
 }
 
 

src/windows/port.cc

@@ -44,12 +44,15 @@
 
 // These call the windows _vsnprintf, but always NUL-terminate.
 #if !defined(__MINGW32__) && !defined(__MINGW64__)  /* mingw already defines */
+#pragma warning(push)
+#pragma warning(disable:4996) // _vsnprintf
 int safe_vsnprintf(char *str, size_t size, const char *format, va_list ap) {
   if (size == 0)        // not even room for a \0?
     return -1;          // not what C99 says to do, but what windows does
   str[size-1] = '\0';
   return _vsnprintf(str, size-1, format, ap);
 }
+#pragma warning(pop)
 
 int snprintf(char *str, size_t size, const char *format, ...) {
   int r;

src/windows/port.h

@@ -65,6 +65,11 @@ extern int GFLAGS_DLL_DECL safe_vsnprintf(char *str, size_t size,
 #define va_copy(dst, src)  (dst) = (src)
 #endif  /* #if !defined(__MINGW32__) && !defined(__MINGW64__) */
 
+#if defined(_MSC_VER) && _MSC_VER >= 1400
+#pragma warning(push)
+#pragma warning(disable:4996) // getenv
+#endif/
+
 inline void setenv(const char* name, const char* value, int) {
   // In windows, it's impossible to set a variable to the empty string.
   // We handle this by setting it to "0" and the NUL-ing out the \0.
@@ -74,6 +79,13 @@ inline void setenv(const char* name, const char* value, int) {
   static const char* const kFakeZero = "0";
   if (*value == '\0')
     value = kFakeZero;
+#if defined(_MSC_VER) && _MSC_VER >= 1400
+  // putenv_s does not need us to malloc anything, therefore no more
+  // leaks for modern MS toolchain.
+  _putenv_s(name, value);
+  if (value == kFakeZero)
+    *getenv(name) = '\0';
+#else
   // Apparently the semantics of putenv() is that the input
   // must live forever, so we leak memory here. :-(
   const int nameval_len = strlen(name) + 1 + strlen(value) + 1;
@@ -85,11 +97,13 @@ inline void setenv(const char* name, const char* value, int) {
     if (*getenv(name) != '\0')
       *getenv(name) = '\0';            // works when putenv() copies nameval
   }
+#endif /* defined(_MSC_VER) && _MSC_VER >= 1400 */
 }
 
 #define strcasecmp _stricmp
 
 #if defined(_MSC_VER) && _MSC_VER >= 1400
+#pragma warning(pop)
 #define strdup   _strdup
 #define unlink   _unlink
 #endif