Fix GDB packet p (readRegister)#1952
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe handling of the "p" command in the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the GDB server's register reading functionality where the p (readRegister) command was not sending results back to the client.
- Fixes the
pcommand to actually send register data back to the GDB client - Wraps the
dumpOneRegister(n)call withwrite()to ensure the result is transmitted
| n <<= 4; | ||
| n |= fromHexChar(m_cmd[2]); | ||
| dumpOneRegister(n); | ||
| write(dumpOneRegister(n)); |
There was a problem hiding this comment.
The change assumes dumpOneRegister(n) returns a value that can be passed to write(), but without seeing the function signature, this could cause a compilation error if dumpOneRegister() returns void or an incompatible type.
| write(dumpOneRegister(n)); | |
| write(std::string(dumpOneRegister(n))); |
dumpOneRegister(n) doesn't actually send the result back, so need to add a write for the result. Tested against an homebrew GDB client.