[Lldb-commits] [lldb] [lldb] gdb rsp file error pass fix (PR #106950)

[David Spickett via lldb-commits]

================
@@ -3064,22 +3064,41 @@ static int gdb_errno_to_system(int err) {
 
 static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response,
                                           uint64_t fail_result, Status &error) {
+  // The packet is expected to have the following format:
+  // 'F<retcode>,<errno>'
+
   response.SetFilePos(0);
   if (response.GetChar() != 'F')
     return fail_result;
+
   int32_t result = response.GetS32(-2, 16);
   if (result == -2)
     return fail_result;
-  if (response.GetChar() == ',') {
-    int result_errno = gdb_errno_to_system(response.GetS32(-1, 16));
-    if (result_errno != -1)
-      error = Status(result_errno, eErrorTypePOSIX);
-    else
-      error = Status(-1, eErrorTypeGeneric);
-  } else
+
+  if (response.GetChar() != ',') {
     error.Clear();
+    return result;
+  }
+
+  // Response packet should contain a error code at the end. This code
+  // corresponds either to the gdb IOFile error code, or to the posix errno.
----------------
DavidSpickett wrote:

Yes it is a bit vague but going by https://lldb.llvm.org/resources/lldbgdbremote.html#vfile-open "the errno" doesn't say to me "the posix errno" necessarily. You are right that for `lldb-server` specifically, it would defacto mean the posix errno, but there is also Windows to consider.

But the code you are editing here is the client side (`lldb`). The client could be connected to a debug server running on anything.

So I don't think you can mark the unknown values `eErrorTypePOSIX` because at this point we don't know that the other end is posix. So that's why I'd go with `eErrorTypeGeneric`.

(I suspect this doesn't really matter in the end, we probably just print the error number the same way either way)

The point of your patch is to include the number instead of overwriting it with -1, and that is fine I understand why that's useful.

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