tberghammer added a comment.
I few high level comments after a quick read through.
================
Comment at: unittests/tools/lldb-server/.gitignore:1
+thread_inferior
----------------
Why do we need this? I would expect cmake to *not* put anything into my source
directory when doing an out-of-source build so I can have multiple build
directory (e.g. for different targets) at the same time.
================
Comment at: unittests/tools/lldb-server/inferior/thread_inferior.cpp:21
+
+ asm volatile ("int3");
+ delay = false;
----------------
krytarowski wrote:
> int3 is specific to x86.
>
> Some instructions to emit software breakpoint in other architectures:
> https://github.com/NetBSD/src/commit/c215d1b7d6c1385720b27fd45189b5dea6d6e4aa
>
> Also there is a support for threads, this is not as portable as it could be.
> The simplest example could be without them, and threads in debuggee could be
> tested in dedicated regression tests. This will help out to sort bugs with
> threads from other features.
I think there should be a compiler intrinsic available as well.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:17
+
+class ProcessInfo {
+public:
----------------
Can we somehow reuse the logic in GDBRemoteCommunicationClient for parsing the
packets (possibly after factoring out some of it into new standalone
functions)? I think it would remove code duplication as well as increase the
coverage provided by these tests.
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:82-87
+// Common functions for parsing packet data.
+std::unordered_map<std::string, std::string> SplitPairList(const std::string&
s);
+std::vector<std::string> SplitList(const std::string& s, char delimeter);
+std::pair<std::string, std::string> SplitPair(const std::string& s);
+std::string HexDecode(const std::string& hex_encoded);
+unsigned long SwitchEndian(const std::string& little_endian);
----------------
Does these have to be global? Can we make them local to the .cc file (anonymous
namespace or file static variable)?
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:83
+// Common functions for parsing packet data.
+std::unordered_map<std::string, std::string> SplitPairList(const std::string&
s);
+std::vector<std::string> SplitList(const std::string& s, char delimeter);
----------------
What if the key isn't unique?
================
Comment at: unittests/tools/lldb-server/tests/MessageObjects.h:86-87
+std::pair<std::string, std::string> SplitPair(const std::string& s);
+std::string HexDecode(const std::string& hex_encoded);
+unsigned long SwitchEndian(const std::string& little_endian);
+}
----------------
I would hope we have functions in LLVM/LLDB doing these sort of things (might
have to be changed slightly to make them easily accessible)
https://reviews.llvm.org/D32930
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
