[Lldb-commits] [lldb] [lldb] Assorted improvements to the Pipe class (PR #128719)

[Pavel Labath via lldb-commits]

================
@@ -300,70 +304,51 @@ void PipePosix::CloseWriteFileDescriptorUnlocked() {
   }
 }
 
-Status PipePosix::ReadWithTimeout(void *buf, size_t size,
-                                  const std::chrono::microseconds &timeout,
-                                  size_t &bytes_read) {
+llvm::Expected<size_t> PipePosix::Read(void *buf, size_t size,
+                                       const Timeout<std::micro> &timeout) {
   std::lock_guard<std::mutex> guard(m_read_mutex);
-  bytes_read = 0;
   if (!CanReadUnlocked())
-    return Status(EINVAL, eErrorTypePOSIX);
+    return llvm::errorCodeToError(
+        std::make_error_code(std::errc::invalid_argument));
 
   const int fd = GetReadFileDescriptorUnlocked();
 
   SelectHelper select_helper;
-  select_helper.SetTimeout(timeout);
+  if (timeout)
+    select_helper.SetTimeout(*timeout);
   select_helper.FDSetRead(fd);
 
-  Status error;
-  while (error.Success()) {
-    error = select_helper.Select();
-    if (error.Success()) {
-      auto result =
-          ::read(fd, static_cast<char *>(buf) + bytes_read, size - bytes_read);
-      if (result != -1) {
-        bytes_read += result;
-        if (bytes_read == size || result == 0)
-          break;
-      } else if (errno == EINTR) {
-        continue;
-      } else {
-        error = Status::FromErrno();
-        break;
-      }
-    }
-  }
-  return error;
+  if (llvm::Error error = select_helper.Select().takeError())
+    return error;
+
+  ssize_t result = ::read(fd, buf, size);
+  if (result == -1)
+    return llvm::errorCodeToError(
+        std::error_code(errno, std::generic_category()));
----------------
labath wrote:

I don't believe that necessary because EINTR is returned when a *blocking* syscall is interrupted, and the select call above basically guarantees that this will not block:
```
       If  a signal handler is invoked while a system call or library function call is blocked, then either:
       •  the call is automatically restarted after the signal handler returns; or
       •  the call fails with the error EINTR.
```

```
       select()  allows a program to monitor multiple file descriptors, waiting until one or more of the
       file descriptors become "ready" for some class of I/O operation (e.g., input possible).   A  file
       descriptor  is considered ready if it is possible to perform a corresponding I/O operation (e.g.,
       read(2), or a sufficiently small write(2)) without blocking.
```

This is based on the linux documentation, but I'd expect other systems to behave the same way. That said, the check doesn't hurt much, so I'd fine with adding it if you think it helps.

https://github.com/llvm/llvm-project/pull/128719