mgorny added inline comments.
================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:661-664 + if (mode != llvm::sys::fs::perms_not_known) + response.Printf("F%x", mode); + else + response.Printf("F-1,%x", (int)Status(ec).GetError()); ---------------- labath wrote: > mgorny wrote: > > labath wrote: > > > Unless I'm mistaken, the test does not actually run this code (as it > > > tests the client bits). > > Yes, this is the case. Adding tests for the server is a bit out of scope > > for what I'm working on. > Why is that? You did have them in the other patches I looked at... > > Batching these changes in this way is particularly unfortunate because a > casual observer might conclude that you actualy _are_ including a test for > the bug you've fixed. Just to be clear, are we talking of adding a generic server-side test for `vFile:mode`, or specifically one that triggers an error? Though thinking about it a bit more, using a file that does not exist should be good enough to trigger this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107809/new/ https://reviews.llvm.org/D107809 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits