jasonmolenda wrote: I chatted with Felipe. I hadn't followed the original RFC very well after it went up and missed the discussion about an `options` key-value. I think people are trying to make this packet more extensible than gdb remote serial packets normally are.
Extensibility can come in two forms: adding additional behaviors, or changing existing behaviors. The gdb remote serial protocol packets with key-value pairs or JSON arguments can add additional behaviors well. They do not handle changing existing behaviors well -- or at all, usually, without adding some additional hint like in `qSupported` or something. An example of where this has been a problem in the past would be the `QEnvironment` packet, which accepted an environment variable to set in a launched process, NAME=VALUE, in plain text. Eventually someone tried to send a name or value with one of the critical gdb remote serial protocol characters like # or $, and it was a big problem. And so we now have `QEnvironmentHexEncoded` which takes asciihex encoded NAME=value. The documentation here (and impementation in the other PR) are trying to use the `options` field to allow for changing existing behavior. For instance, the current packet uses the gdb remote serial protocol binary escape scheme, where the channel is assumed to be 8 bit clean and we need to escape 4 characters. If we wanted to use this packet over a channel that was only 7 bit clean (this will not happen), we would need to change the packet to send its read bytes in asciihex instead of the binary escape protocol -- a fundamental change in the initial implementation behavior. The scheme as implemented right now, is that if a debugger sends an `options:` key-value pair with any value in it, an existing stub will error out, assuming that a change in behavior is required. In short, this is a versioning field. If we want to do this, we might as well stick a `version:1` key-value in the packet. We've never done this for a gdb remote serial protocol packet, but if we think there is a need/value in doing this, I'd say let's make it obvious what it is. And as a far less important aside, don't hardcode the order of key-values. It's a dictionary/map/hash/KV array, order is not significant. https://github.com/llvm/llvm-project/pull/162675 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
