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

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:1138
+      // Double quote the string.
+      ::snprintf(arg_cstr, sizeof(arg_cstr), "--log-channels=\"%s\"",
+                 env_debugserver_log_channels.c_str());
----------------
Hui wrote:
> This change is essential others seem not that necessary but nice to have in order to support multiple systems.
I would prefer to avoid using C-style formatting if possible.  LLVM has some nice formatting routines that are safer and easier to understand.

While we're here, perhaps we could change `char arg_cstr[PATH_MAX];` up at the top of this function to `std::string arg_str;`

Regardless, we can re-write this in a number of ways, such as:
```
arg_str = llvm::formatv("--log-channels = \"{0}\", env_debugserver_log_channels).str();
arg_str = (Twine("--log-channels = \"") + env_debugserver_log_channels + "\"").str();
arg_str = llvm::format("--log-channels = \"%s\"", env_debugserver_log_channels);
```

to get rid of this snprintf call.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D56230