[Lldb-commits] [PATCH] D32930: New framework for lldb client-server communication tests.

Jason Majors via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri May 12 15:53:42 PDT 2017

jmajors marked 6 inline comments as done.
jmajors added inline comments.

Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:25-30
+  elements["pid"].getAsInteger(16, pid);
+  elements["parent-pid"].getAsInteger(16, parent_pid);
+  elements["real-uid"].getAsInteger(16, real_uid);
+  elements["real-gid"].getAsInteger(16, real_gid);
+  elements["effective-uid"].getAsInteger(16, effective_uid);
+  elements["effective-gid"].getAsInteger(16, effective_gid);
zturner wrote:
> Since this is in a constructor, what happens if you get a malformed response?  You don't have a way to return an error here, and none of these errors are checked.  Do you want to `assert` if this happens?  Or are you ok silently dropping this kind of failure?
I changed it to a Create with tests for success.

Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:142
+    llvm::support::endianness endian) {
+  auto stop_reply = std::shared_ptr<StopReply>(new StopReply());
zturner wrote:
> Can you use `llvm::make_shared<StopReply>()` here?  Probably doesn't matter much, but it's good practice to use `make_shared<>` wherever possible since it uses less memory.
I wanted to keep my constructors protected, to force the use of Create(). make_shared doesn't work with protected constructors.

Comment at: unittests/tools/lldb-server/tests/MessageObjects.cpp:167
+    std::stringstream ss;
+    ss << std::hex << std::setw(2) << std::setfill('0') << register_id;
+    std::string hex_id = ss.str();
zturner wrote:
> Is this the same as `std::string hex_id = llvm::utostr(register_id);`?
No. The register IDs are all two nibbles wide, so I need the setw & setfill (or equivalent).


More information about the lldb-commits mailing list