[Lldb-commits] [lldb] [lldb] Ignore async notification packets while waiting for a response (PR #202556)

[Jason Molenda via lldb-commits]

jasonmolenda wrote:

I apologize for the delay in responding, I am wrong and this is correct.  When we get one or more notify packets but no actual response, we will read it/them with
```
    size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
```

then iterate over the received packet(s) with one or more calls to `CheckForPacket`.  The final of which will find nothing in the incoming buffer and return `PacketType::Invalid`.  That puts us in

```
    } else {
      switch (status) {
      case eConnectionStatusSuccess:
        // printf ("status = success but error = %s\n",
        // error.AsCString("<invalid>"));
        break;
```

and we loop again to
```
  while (IsConnected() && !timed_out) {
    size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
```

and wait for the next notify packet, or real response.  

Could you please add a comment to this `case eConnectionStatusSuccess` explaining that we received a notify packet only; we don't have the actual response yet so we need to continue looping for it.  This case is important with notify packets, and the existing comment might lead someone to wonder why it exists. 

And thanks for adding the test case that sends a notify, then sends the actual reply separately - that will detect if anyone removes this `case` statement in the future too.

This PR looks good to me, I'd like to see a comment on that `case` statement but that's my only feedback.  Thanks!

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