labath added a comment.
Including the args sounds like a good idea, but I don't think the chosen
encoding scheme is very good. The encoding done in `GetCommandString` is very
naive and not reversible. Since you're hex-encoding the result anyway, and the
nul character cannot be present inside an argument you could use it to delimit
the individual arguments (i.e. `666f6f00626172` -> `{"foo", "bar"}`). Or, you
could look at how the `$A` packet transmits arguments, and implement something
similar.
Also, you should write a test for this. I think it should be possible to run a
process with known arguments, and then verify that they show up in "platform
process list" output. Alternatively, the client part of the protocol can also
be tested via c++ unit tests (see unittests/Process/gdb-remote). The advantage
of that approach is that you're able to inject invalid packets and test the
handling of those. In an ideal world, we'd have both kinds of these tests, but
we're pretty far from that, so I'd settle for at least one of them. :)
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:613-615
+ if (SendPacketAndWaitForResponse(
+ "jGetLoadedDynamicLibrariesInfos:", response, false) ==
+ PacketResult::Success) {
----------------
When you're running clang format, please make sure it only formats the lines
that you have modified. I've you're using the standard git-clang-format
integration, this should be as simple as `git clang-format HEAD^`. Please
revert these unrelated changes.
================
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:1944-1948
+ if (!process_info.GetExecutableFile().GetPath().empty()) {
+ process_info.SetArguments(
+ Args(process_info.GetExecutableFile().GetPath() + args_command),
+ true);
+ }
----------------
I don't fully understand the what this does. Is it trying to set the executable
name as argv[0]? Wouldn't it be better to send the argv[0] separately (together
with the rest of the args array), just in case the process has a special
argv[0].
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68293/new/
https://reviews.llvm.org/D68293
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
