[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

labath added a comment. I think this is fine, except for the things I pointed out inline. As for `SafelyCloseFileDescriptor` (wow) thing, I think we should use that, given that it's already there. I suggest making a separate patch for that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

labath added inline comments. Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:255-258 +llvm::sys::RetryAfterSignal(-1, ::cfsetospeed, +&options, B115200); +llvm::sys::RetryAfterSignal(-1, ::cfsetispeed, +&options, B1152

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

mgorny updated this revision to Diff 191641. mgorny added a comment. Covered more callsites. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59606/new/ https://reviews.llvm.org/D59606 Files: lldb/source/Host/common/PseudoTerminal.cpp lldb/source/Host/common/Socket.cpp lldb/source/H

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

clayborg added a comment. LGTM as long as the test suite is happy CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59606/new/ https://reviews.llvm.org/D59606 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

mgorny updated this revision to Diff 191553. mgorny added a comment. Got rid of `close()` and `fclose()` wrapping, for now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59606/new/ https://reviews.llvm.org/D59606 Files: lldb/source/Host/common/Socket.cpp lldb/source/Host/common/TCP

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

mgorny added a comment. Hmm, I also see that LLVM has signal-safe `Process::SafelyCloseFileDescriptor()`. Should I use that, or just ignore potential issues with `close()`? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59606/new/ https://reviews.llvm.org/D596

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

mgorny added a comment. In D59606#1436796 , @labath wrote: > The "problem" I have with this is that (IIRC) retrying close(2) on EINTR is > the *wrong* thing to do on linux (because the fd will be closed anyway, and > so we may end up closing someone else

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

labath added a comment. The "problem" I have with this is that (IIRC) retrying close(2) on EINTR is the *wrong* thing to do on linux (because the fd will be closed anyway, and so we may end up closing someone else's file the second time around). I'll try to dig up more info about that tomorrow.

[Lldb-commits] [PATCH] D59606: [lldb] [WIP/RFC] Add missing retries on EINTR

mgorny created this revision. mgorny added reviewers: labath, JDevlieghere, zturner. Herald added subscribers: ki.stfu, emaste. Herald added a project: LLDB. This is WIP on adding EINTR handling all over the place, via `llvm::sys::RetryAfterSignal()`. If you don't like this approach, please lemme