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). https://reviews.llvm.org/D32930 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits