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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
