[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)

[Hu Jialun via lldb-commits]

================
@@ -24,41 +30,95 @@ using namespace llvm;
 
 namespace lldb_dap {
 
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
-
+FifoFile::FifoFile(StringRef path)
+    : m_path(path), m_file(fopen(path.data(), "r+")) {
+  std::error_code EC;
+  if (m_file == nullptr) {
+    EC = std::error_code(errno, std::generic_category());
+    llvm::errs() << "Failed to open fifo file " << path << ": " << EC.message()
+                 << "\n";
+    std::terminate();
+  }
+  if (setvbuf(m_file, NULL, _IONBF, 0))
+    llvm::errs() << "Error setting unbuffered mode on C FILE\n";
+}
+FifoFile::FifoFile(StringRef path, FILE *f) : m_path(path), m_file(f) {}
+FifoFile::FifoFile(FifoFile &&other)
+    : m_path(other.m_path), m_file(other.m_file) {
+  other.m_path.clear();
+  other.m_file = nullptr;
+}
 FifoFile::~FifoFile() {
+  if (m_file)
+    fclose(m_file);
 #if !defined(_WIN32)
+  // Unreferenced named pipes are deleted automatically on Win32
   unlink(m_path.c_str());
 #endif
 }
 
-Expected<std::shared_ptr<FifoFile>> CreateFifoFile(StringRef path) {
-#if defined(_WIN32)
-  return createStringError(inconvertibleErrorCode(), "Unimplemented");
+// This probably belongs to llvm::sys::fs as another FSEntity type
+std::error_code createUniqueNamedPipe(const Twine &prefix, StringRef suffix,
+                                      int &result_fd,
+                                      SmallVectorImpl<char> &result_path) {
+  const char *middle = suffix.empty() ? "-%%%%%%" : "-%%%%%%.";
+  auto EC = sys::fs::getPotentiallyUniqueFileName(
+#ifdef _WIN32
+      "\\\\.\\pipe\\LOCAL\\"
+#else
+      "/tmp/"
+#endif
+          + prefix + middle + suffix,
+      result_path);
+  if (EC)
+    return EC;
+  result_path.push_back(0);
+  const char *path = result_path.data();
+#ifdef _WIN32
+  HANDLE h = ::CreateNamedPipeA(
+      path, PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE,
+      PIPE_TYPE_BYTE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, 1024, 1024, 0, NULL);
+  if (h == INVALID_HANDLE_VALUE)
+    return llvm::mapLastWindowsError();
+  result_fd = _open_osfhandle((intptr_t)h, _O_TEXT | _O_RDWR);
+  if (result_fd == -1)
+    return llvm::mapLastWindowsError();
 #else
-  if (int err = mkfifo(path.data(), 0600))
-    return createStringError(std::error_code(err, std::generic_category()),
-                             "Couldn't create fifo file: %s", path.data());
-  return std::make_shared<FifoFile>(path);
+  if (mkfifo(path, 0600) == -1)
+    return std::error_code(errno, std::generic_category());
+  EC = openFileForWrite(result_path, result_fd, sys::fs::CD_OpenExisting,
+                        sys::fs::OF_None, 0600);
+  if (EC)
+    return EC;
 #endif
+  result_path.pop_back();
+  return std::error_code();
 }
 
-FifoFileIO::FifoFileIO(StringRef fifo_file, StringRef other_endpoint_name)
-    : m_fifo_file(fifo_file), m_other_endpoint_name(other_endpoint_name) {}
+FifoFileIO::FifoFileIO(FifoFile &&fifo_file, StringRef other_endpoint_name)
+    : m_fifo_file(std::move(fifo_file)),
+      m_other_endpoint_name(other_endpoint_name) {}
 
 Expected<json::Value> FifoFileIO::ReadJSON(std::chrono::milliseconds timeout) {
   // We use a pointer for this future, because otherwise its normal destructor
   // would wait for the getline to end, rendering the timeout useless.
   std::optional<std::string> line;
   std::future<void> *future =
       new std::future<void>(std::async(std::launch::async, [&]() {
-        std::ifstream reader(m_fifo_file, std::ifstream::in);
-        std::string buffer;
-        std::getline(reader, buffer);
-        if (!buffer.empty())
-          line = buffer;
+        rewind(m_fifo_file.m_file);
+        constexpr size_t buffer_size = 2048;
+        char buffer[buffer_size];
+        char *ptr = fgets(buffer, buffer_size, m_fifo_file.m_file);
+        if (ptr == nullptr || *ptr == 0)
+          return;
+        size_t len = strlen(buffer);
+        if (len <= 1)
+          return;
+        buffer[len - 1] = '\0'; // remove newline
+        line = buffer;
----------------
SuibianP wrote:

We can reinvent POSIX `getline`, but it is probably not worth the effort. Any JSON schema change that overruns the buffer will absolutely get truncated, break the test visibly and get caught during review.

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