[Lldb-commits] [PATCH] D59606: [lldb] Add missing EINTR handling

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:255-258
+        llvm::sys::RetryAfterSignal(-1, ::cfsetospeed,
+            &options, B115200);
+        llvm::sys::RetryAfterSignal(-1, ::cfsetispeed,
+            &options, B115200);
----------------
mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > labath wrote:
> > > > IIUC, these only manipulate the `options` struct, so I don't see how they could fail with EINTR. OTOH, the `tcsetattr` call below is documented to return EINTR at least on NetBSD <http://netbsd.gw.com/cgi-bin/man-cgi?cfsetospeed+3+NetBSD-current>.
> > > Yeah, that's why I covered it, though it looks surprising to me as well. But looking at the code, it can't happen really, so I'll just revert this.
> > I think we misunderstood each other. My expectation would be that these two calls don't need EINTR protection, but the tcsetattr call (line 267) does.
> I'm not sure if it can happen with `TCSANOW` but I guess better safe than sorry.
Ah, OK, I see. I am not really familiar with this syscall, so if you are reasonably confident that this particular combo of arguments cannot block (and be interrupted), then I'm fine with leaving the EINTR loop out.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59606/new/

https://reviews.llvm.org/D59606