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

[John Harrison 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;
----------------
ashgti wrote:

The previous version would get a full line, however this new version will read until the buffer is full or EOF. The JSON used is smaller than the buffer size, so this should be okay, but if the JSON value was changed then this could be an issue.

Should we read until EOF? Or Should we read until we find a newline specifically?

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