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

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 15 11:01:21 PST 2019

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



More information about the lldb-commits mailing list