Timmmm wrote:

Yes.... that PR looks like it is just AI, and it doesn't fix the issue that 
@DavidSpickett identified. Honestly I'll be a bit disappointed if the 
auto-generated AI PR gets merged for a bug that I discovered, reported and 
submitted a fix for.

I guess I can't entirely blame them because I only said I have a fix in [a 
different 
PR](https://github.com/llvm/llvm-project/pull/203204#issuecomment-4678948910) 
and I can't expect them to see that, but still.

> But yeah, the problem is that lldb can receive a single packet with a Read(), 
> it can receive multiple packets with a Read(), or we can probably receive 
> parts of a packet in a Read()

The existing code works fine, as long as you never get a single `Read()` that 
returns one or more ignored packets followed by a complete valid packet. As you 
said corrupted packets are going to be extremely rare in practice so it 
probably doesn't really matter too much for those, but now that "ignored 
packets" ignores notifications it does matter for those.

The clean fix would be to change `CheckPacket()` so that it can return 
`Incomplete` instead of `Invalid` for situations where there isn't enough data. 
But I think that should be in a separate PR.

https://github.com/llvm/llvm-project/pull/204788
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to