labath accepted this revision. labath added a comment. This revision is now accepted and ready to land.
LGTM, modulo the inline comments. ================ Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:25-26 // NativeProcessProtocol Members +const size_t NativeProcessProtocol::g_cache_line_size = + llvm::sys::Process::getPageSizeEstimate(); ---------------- Make this a local static in `ReadCStringFromMemory`. There's no need to execute this immediately on program startup.. ================ Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:100 + +TEST(NativeProcessProtocolTest, ReadCStringFromMemory) { + NiceMock<MockDelegate> DummyDelegate; ---------------- labath wrote: > It would be nice to have one more test where reads cross the page boundary, > to make sure chunks are concatenated properly. How about this? I still think it would be useful to have it, as the cross-page handling is the main source of complexity in this function. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62503/new/ https://reviews.llvm.org/D62503 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits