Add Utility/Environment class for handling... environments

Summary:
There was some confusion in the code about how to represent process
environment. Most of the code (ab)used the Args class for this purpose,
but some of it used a more basic StringList class instead. In either
case, the fact that the underlying abstraction did not provide primitive
operations for the typical environment operations meant that even a
simple operation like checking for an environment variable value was
several lines of code.

This patch adds a separate Environment class, which is essentialy a
llvm::StringMap<std::string> in disguise. To standard StringMap
functionality, it adds a couple of new functions, which are specific to
the environment use case:
- (most important) envp conversion for passing into execve() and likes.
  Instead of trying to maintain a constantly up-to-date envp view, it
  provides a function which creates a envp view on demand, with the
  expectation that this will be called as the very last thing before
  handing the value to the system function.
- insert(StringRef KeyEqValue) - splits KeyEqValue into (key, value)
  pair and inserts it into the environment map.
- compose(value_type KeyValue) - takes a map entry and converts in back
  into "KEY=VALUE" representation.

With this interface most of the environment-manipulating code becomes
one-liners. The only tricky part was maintaining compatibility in
SBLaunchInfo, which expects that the environment entries are accessible
by index and that the returned const char* is backed by the launch info
object (random access into maps is hard and the map stores the entry in
a deconstructed form, so we cannot just return a .c_str() value). To
solve this, I have the SBLaunchInfo convert the environment into the
"envp" form, and use it to answer the environment queries. Extra code is
added to make sure the envp version is always in sync.

(This also improves the layering situation as Args was in the Interpreter module
whereas Environment is in Utility.)

Reviewers: zturner, davide, jingham, clayborg

Subscribers: emaste, lldb-commits, mgorny

Differential Revision: https://reviews.llvm.org/D41359

llvm-svn: 322174

1 files changed, 24 insertions, 39 deletions

diff --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index fdc10cf4827..1464d4c999c 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -3611,36 +3611,19 @@ protected:
                 nullptr, idx, g_properties[idx].default_uint_value != 0)) {
           PlatformSP platform_sp(m_target->GetPlatform());
           if (platform_sp) {
-            StringList env;
-            if (platform_sp->GetEnvironment(env)) {
-              OptionValueDictionary *env_dict =
-                  GetPropertyAtIndexAsOptionValueDictionary(nullptr,
-                                                            ePropertyEnvVars);
-              if (env_dict) {
-                const bool can_replace = false;
-                const size_t envc = env.GetSize();
-                for (size_t idx = 0; idx < envc; idx++) {
-                  const char *env_entry = env.GetStringAtIndex(idx);
-                  if (env_entry) {
-                    const char *equal_pos = ::strchr(env_entry, '=');
-                    ConstString key;
-                    // It is ok to have environment variables with no values
-                    const char *value = nullptr;
-                    if (equal_pos) {
-                      key.SetCStringWithLength(env_entry,
-                                               equal_pos - env_entry);
-                      if (equal_pos[1])
-                        value = equal_pos + 1;
-                    } else {
-                      key.SetCString(env_entry);
-                    }
-                    // Don't allow existing keys to be replaced with ones we get
-                    // from the platform environment
-                    env_dict->SetValueForKey(
-                        key, OptionValueSP(new OptionValueString(value)),
-                        can_replace);
-                  }
-                }
+            Environment env = platform_sp->GetEnvironment();
+            OptionValueDictionary *env_dict =
+                GetPropertyAtIndexAsOptionValueDictionary(nullptr,
+                                                          ePropertyEnvVars);
+            if (env_dict) {
+              const bool can_replace = false;
+              for (const auto &KV : env) {
+                // Don't allow existing keys to be replaced with ones we get
+                // from the platform environment
+                env_dict->SetValueForKey(
+                    ConstString(KV.first()),
+                    OptionValueSP(new OptionValueString(KV.second.c_str())),
+                    can_replace);
               }
             }
           }
@@ -3906,15 +3889,19 @@ void TargetProperties::SetRunArguments(const Args &args) {
   m_launch_info.GetArguments() = args;
 }
 
-size_t TargetProperties::GetEnvironmentAsArgs(Args &env) const {
+Environment TargetProperties::GetEnvironment() const {
+  // TODO: Get rid of the Args intermediate step
+  Args env;
   const uint32_t idx = ePropertyEnvVars;
-  return m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env);
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env);
+  return Environment(env);
 }
 
-void TargetProperties::SetEnvironmentFromArgs(const Args &env) {
+void TargetProperties::SetEnvironment(Environment env) {
+  // TODO: Get rid of the Args intermediate step
   const uint32_t idx = ePropertyEnvVars;
-  m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, env);
-  m_launch_info.GetEnvironmentEntries() = env;
+  m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, Args(env));
+  m_launch_info.GetEnvironment() = std::move(env);
 }
 
 bool TargetProperties::GetSkipPrologue() const {
@@ -4152,7 +4139,7 @@ void TargetProperties::SetProcessLaunchInfo(
   m_launch_info = launch_info;
   SetArg0(launch_info.GetArg0());
   SetRunArguments(launch_info.GetArguments());
-  SetEnvironmentFromArgs(launch_info.GetEnvironmentEntries());
+  SetEnvironment(launch_info.GetEnvironment());
   const FileAction *input_file_action =
       launch_info.GetFileActionForFD(STDIN_FILENO);
   if (input_file_action) {
@@ -4193,9 +4180,7 @@ void TargetProperties::EnvVarsValueChangedCallback(void *target_property_ptr,
                                                    OptionValue *) {
   TargetProperties *this_ =
       reinterpret_cast<TargetProperties *>(target_property_ptr);
-  Args args;
-  if (this_->GetEnvironmentAsArgs(args))
-    this_->m_launch_info.GetEnvironmentEntries() = args;
+  this_->m_launch_info.GetEnvironment() = this_->GetEnvironment();
 }
 
 void TargetProperties::InputPathValueChangedCallback(void *target_property_ptr,