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

[Pavel Labath via lldb-commits]

================
@@ -1154,17 +1156,25 @@ Status GDBRemoteCommunication::StartDebugserverProcess(
       if (socket_pipe.CanWrite())
         socket_pipe.CloseWriteFileDescriptor();
       if (socket_pipe.CanRead()) {
-        // The port number may be up to "65535\0".
-        char port_cstr[6] = {0};
-        size_t num_bytes = sizeof(port_cstr);
         // Read port from pipe with 10 second timeout.
-        error = socket_pipe.ReadWithTimeout(
-            port_cstr, num_bytes, std::chrono::seconds{10}, num_bytes);
+        std::string port_str;
+        while (error.Success()) {
+          char buf[10];
+          if (llvm::Expected<size_t> num_bytes = socket_pipe.Read(
+                  buf, std::size(buf), std::chrono::seconds(10))) {
+            port_str.append(buf, *num_bytes);
+            if (*num_bytes == 0)
----------------
labath wrote:

I'll move the zero check.

I can't really call `reserve()` on the string as writing past the `end()` of the string is still illegal (and the data would be later overwritten when i set the "real" end value). I could `resize()` the string then read directly into it, but then I'd have keep track of the total number of bytes I've read separately and do index arithmetic and whatnot. This implementation just reads into `buf` over and over again (this also explains why reading more than buf size is not a problem) and then appends that data into the string. It's not particularly efficient, but I thought it would be fine given that this is reading a short string and this loop will approximately never run more than once.

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