[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments

Greg Clayton via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Mon Dec 18 10:09:47 PST 2017

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

ok as long as we don't want to return const reference when returning Environment values in getters and setters.

Comment at: include/lldb/Target/Platform.h:636
-  virtual size_t GetEnvironment(StringList &environment);
+  virtual Environment GetEnvironment();
Do we want to return by value? Not a const reference?

Comment at: include/lldb/Target/Target.h:118-119
-  size_t GetEnvironmentAsArgs(Args &env) const;
-  void SetEnvironmentFromArgs(const Args &env);
+  Environment GetEnvironment() const;
+  void SetEnvironment(Environment env);
set/get using reference instead of by value?

Comment at: source/API/SBLaunchInfo.cpp:34
+  Environment::Envp m_envp;
Seems like a lot of work to get indexed environment variable access. Should we not make the Environment class use a std::vector instead of a map? Not sure if environment variable order is ever important? Is so then the StringMap isn't what we want to use. I know we have it in our public API so we can't change it now and thus why we need this work around. Just thinking out loud


More information about the lldb-commits mailing list