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

[Hui Huang via Phabricator via lldb-commits]

Hui added a comment.

In D56230#1361634 <https://reviews.llvm.org/D56230#1361634>, @labath wrote:

> In D56230#1358356 <https://reviews.llvm.org/D56230#1358356>, @zturner wrote:
>
> > I've always disliked this argument and hoped that someday someone would remove it entirely.  My recollection (which may be wrong) is that the only actual use of it is so that if someone types a command, and we later need to print the command back, we will print it with the same quote char.  It almost seems like we could just delete the argument and use a standardized quote char when flattening a command string.
>
>
> +100
>
> BTW, today I've tried to switch ProcessLauncherWindows to `flattenWindowsCommandLine` and this change alone was enough to fix `TestQuoting`, which has some tests XFAILed for windows due to quoting problems. I haven't sent out a patch yet because that has also broken `platform shell dir c:\` for some reason, and I haven't gotten around to investigating that.  If you (for any value of you) have some time, I'd encourage you to look into that. Otherwise, I'll get to that eventually.


Yes. I myself added a patch to test quoting also launch gdbserver by lldb-server.exe. Ran well so far. Also lldb/unittests/Utility tests.

Haven't tried other regression tests yet.

+#if defined(_WIN32)
+TEST(ArgsTest, GetFlattenWindowsCommandString) {
+  Args args;
+  args.AppendArgument("D:\\launcher.exe");
+  args.AppendArgument("--log=abc def");
+
+  std::string stdstr;
+  ASSERT_TRUE(args.GetFlattenWindowsCommandString(stdstr));
+  EXPECT_EQ(stdstr, "\"D:\\launcher.exe\" \"--log=abc def\"");
+}
+#endif


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

https://reviews.llvm.org/D56230