Move AlterEnvironment to base/environment.h, implement on Windows.

base/environment.cc

Index: base/environment.cc
diff --git a/base/environment.cc b/base/environment.cc
index c1f2008b4ce31951a32ac2414c454806048cea72..4082b907f20690bc6e6a4cc8931d5cd3c23d68f2 100644
--- a/base/environment.cc
+++ b/base/environment.cc
@@ -4,18 +4,19 @@
 
 #include "base/environment.h"
 
+#include <map>

[viettrungluu, 2013/08/09 19:27:11]

I don't think this include is needed (given that it's included and required by
the header, and you have no standalone uses of std::map).

On the other hand, you should probably include <vector>.

+
+#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+
 #if defined(OS_POSIX)
 #include <stdlib.h>
 #elif defined(OS_WIN)
 #include <windows.h>
 #endif
 
-#include "base/strings/string_util.h"
-
-#if defined(OS_WIN)
-#include "base/memory/scoped_ptr.h"
-#include "base/strings/utf_string_conversions.h"
-#endif
+namespace base {
 
 namespace {
 
@@ -99,9 +100,24 @@ class EnvironmentImpl : public base::Environment {
   }
 };
 
-}  // namespace
+// Parses a null-terminated input string of an environment block. The key is
+// placed into the given string, and the total length of the line, including
+// the terminating null, is returned.
+size_t ParseEnvLine(const NativeEnvironmentString::value_type* input,
+                    NativeEnvironmentString* key) {
+  // Skip to the equals or end of the string, this is the key.
+  size_t cur = 0;
+  while (input[cur] && input[cur] != '=')
+    cur++;
+  *key = NativeEnvironmentString(&input[0], cur);
+
+  // Now just skip to the end of the string.
+  while (input[cur])
+    cur++;
+  return cur + 1;
+}
 
-namespace base {
+}  // namespace
 
 namespace env_vars {
 
@@ -124,4 +140,95 @@ bool Environment::HasVar(const char* variable_name) {
   return GetVar(variable_name, NULL);
 }
 
+#if defined(OS_WIN)
+
+string16 AlterEnvironment(const wchar_t* env,
+                          const EnvironmentMap& changes) {
+  string16 result;
+
+  // First copy all unmodified values to the output.
+  size_t cur_env = 0;
+  string16 key;
+  while (env[cur_env]) {
+    const wchar_t* line = &env[cur_env];
+    size_t line_length = ParseEnvLine(line, &key);
+
+    // Keep only values not specified in the change vector.
+    EnvironmentMap::const_iterator found_change = changes.find(key);
+    if (found_change == changes.end())
+      result.append(line, line_length);
+
+    cur_env += line_length;
+  }
+
+  // Now append all modified and new values.
+  for (EnvironmentMap::const_iterator i = changes.begin();
+       i != changes.end(); ++i) {
+    if (!i->second.empty())
+      result.append(i->first);
+      result.push_back('=');
+      result.append(i->second);
+      result.push_back(0);
+    }
+  }
+
+  // An additional null marks the end of the list.
+  result.push_back(0);
+  return result;
+}
+
+#elif defined(OS_POSIX)
+
+scoped_ptr<char*[]> AlterEnvironment(const char* const* const env,
+                                     const EnvironmentMap& changes) {
+  std::string value_storage;  // Holds concatenated null-terminated strings.
+  std::vector<size_t> result_indices;  // Line indices into value_storage.
+
+  // First build up all of the unchanged environment strings. These are
+  // null-terminated of the form "key=value".
+  std::string key;
+  for (size_t i = 0; env[i]; i++) {
+    size_t line_length = ParseEnvLine(env[i], &key);
+
+    // Keep only values not specified in the change vector.
+    EnvironmentMap::const_iterator found_change = changes.find(key);
+    if (found_change == changes.end()) {
+      result_indices.push_back(value_storage.size());
+      value_storage.append(env[i], line_length);
+    }
+  }
+
+  // Now append all modified and new values.
+  for (EnvironmentMap::const_iterator i = changes.begin();
+       i != changes.end(); ++i) {
+    if (!i->second.empty()) {
+      result_indices.push_back(value_storage.size());
+      value_storage.append(i->first);
+      value_storage.push_back('=');
+      value_storage.append(i->second);
+      value_storage.push_back(0);
+    }
+  }
+
+  // The returned memory needs enough room for the pointer array with a null
+  // terminator, plus the storage for the strings.
+  char* buffer = new char[
+      (result_indices.size() + 1) * sizeof(char*) + value_storage.size()];
+  scoped_ptr<char*[]> result(reinterpret_cast<char**>(buffer));

[viettrungluu, 2013/08/09 19:27:11]

Hrm, putting a char[] into a scoped_ptr<char*[]> seems dubious to me. In
particular, it seems that on destruction of the scoped_ptr, the wrong destructor
is called.

Also, at least theoretically, buffer may not be compatible with the alignment
requirements of char*. (In C++11, you might DCHECK it versus alignof(char*).)

On the other hand, allocating the buffer as an array of char*s (big enough to
contain that chars at the end) is probably kosher.

[brettw, 2013/08/09 20:08:56]

Done.

+
+  // The string storage goes after the array of pointers.
+  char* storage_data = reinterpret_cast<char*>(
+      &result.get()[result_indices.size() + 1]);
+  memcpy(storage_data, value_storage.c_str(), value_storage.size());

[viettrungluu, 2013/08/09 19:27:11]

c_str -> data

[brettw, 2013/08/09 20:08:56]

I did this change but I added an if(). I did the original deliberately because
data() asserts if it's empty, and c_str doesn't.

+
+  // Fill array of pointers at the beginning of the result.
+  for (size_t i = 0; i < result_indices.size(); i++)
+    result[i] = &storage_data[result_indices[i]];
+  result[result_indices.size()] = 0;  // Null terminator.
+
+  return result.Pass();
+}
+
+#endif  // OS_POSIX
+
 }  // namespace base