amccarth accepted this revision.
amccarth added a comment.
This revision is now accepted and ready to land.
The code is correct, so I'm approving, but I suggest a couple fixes to the
comments before committing..
================
Comment at: lldb/source/Host/windows/ProcessLauncherWindows.cpp:27
+ // Environment buffer is a list of null-terminated list of strings. It ends
+ // with a double null.
for (const auto &KV : env) {
----------------
You have an extra "list of" between null-terminated and "strings". And I think
we should point out that it's UTF-16 encoded, so maybe:
```
// The buffer is a list of null-terminated UTF-16 strings, followed by an
// extra L'\0' (two bytes of 0). An empty environment must have one
// empty string, followed by an extra L'\0'.
```
================
Comment at: lldb/source/Host/windows/ProcessLauncherWindows.cpp:39
buffer.push_back(0);
+ // If the environment was empty, we must insert an extra byte to ensure a
+ // double null.
----------------
"... an extra **two** bytes ..."
================
Comment at: lldb/source/Host/windows/ProcessLauncherWindows.cpp:41
+ // double null.
+ if (env.empty()) {
+ buffer.push_back(0);
----------------
There would be no harm in always adding the extra terminator.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76835/new/
https://reviews.llvm.org/D76835
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
