[Lldb-commits] [PATCH] D56230: [gdb-remote] Use lldb's portable Host::GetEnvironment() instead of getenv

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D56230#1358102 <https://reviews.llvm.org/D56230#1358102>, @Hui wrote:

> What do you think of the following codes to be added in Args?
>
>   bool Args::GetFlattenQuotedCommandString(std::string &command) const {
>     std::vector<llvm::StringRef> args_ref;
>     std::vector<std::string> owner;
>  
>     for (size_t i = 0; i < m_entries.size(); ++i) {
>       if (m_entries[i].quote) {
>         std::string arg;
>         arg += m_entries[i].quote;
>         arg += m_entries[i].ref;
>         arg += m_entries[i].quote;
>         owner.push_back(arg);
>         args_ref.push_back(owner.back());
>       } else {
>         args_ref.push_back(m_entries[i].ref);
>       }
>     }
>  
>     command = llvm::sys::flattenWindowsCommandLine(args_ref);
>  
>     return !m_entries.empty();
>   }
>


Yes, that's more-or-less what I had in mind. I have some comments about the implementation, but none of them are fundamental, I think:

- add "windows" to the function name somewhere (`GetFlattenedWindowsCommandString`?, `GetWindowsCommandString`?, ...), as function uses quoting style which is specifically meant for windows, and is unlikely to be correct elsewhere
- drop the `if (m_entries[i].quote)` branch. You don't need it here, and I don't believe it will be correct anyway, because all it will do is cause `llvm::sys::flattenWindowsCommandLine` to add one more quote level around that and escape the quotes added by yourself


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56230/new/

https://reviews.llvm.org/D56230