jasonmolenda wrote:
Hi, thanks for working on this issue.
I'm a little uncertain of the structure of the change in
`GDBRemoteCommunication::WaitForPacketNoLock`. The code as it exists today is
```
uint8_t buffer[8192];
while (IsConnected() && !timed_out) {
size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
if (bytes_read > 0) {
if (CheckForPacket(buffer, bytes_read, packet) != PacketType::Invalid)
return PacketResult::Success;
} else {
// PacketType::Invalid
case eConnectionStatusTimedOut:
timed_out = true;
```
So we read up-to 8192 bytes, or we stop when a timeout has been reached. If we
got any data, we pass it in to `CheckForPacket` which appends it to the
incoming-byte-buffer. We may be receiving more than 8192 bytes worth of
packet, so we continue looping here, getting the next 8192 bytes and adding it
to the internal incoming-packet buffer until we get a complete packet.
This PR changes this loop to read
```
size_t bytes_read = Read(buffer, sizeof(buffer), timeout, status, &error);
if (bytes_read > 0) {
packet_type = CheckForPacket(buffer, bytes_read, packet);
while (packet_type == PacketType::Notify)
packet_type = CheckForPacket(nullptr, 0, packet);
```
The `Read()` receives an async-notify packet. `CheckForPacket` appends it to
the internal buffer, sees that we have a complete packet, returns
`PacketType::Notify`. Now we check if there is *another* async-notify packet,
or real response, that we received at the same time in that original `Read`.
But what if it hasn't been sent yet? I'm guessing that a stub doing this might
send an async-notify packet every 500ms or something, so we'll only receive the
first one in that first `Read`, the `while` loop exits because
`CheckForPacket()` returns
```
packet.Clear();
return GDBRemoteCommunication::PacketType::Invalid;
```
when the incoming packet buffer (`m_bytes.empty()`) is empty. This exits the
`while (packet_type == PacketType::Notify)` loop and we return a failure.
I think the only way this works is in the style of the unit test,
```
ASSERT_TRUE(Write("%oocd_keepalive:01#55%oocd_keepalive:02#56$OK#9a"));
```
where one or more async-notifies AND the final actual packet result are all
received together.
https://github.com/llvm/llvm-project/pull/202556
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
