Review Request 121721: gdb: port BreakpointController to the new interface, handle async breakpoint notifications

Aleix Pol Gonzalez aleixpol at kde.org
Mon Dec 29 00:21:41 UTC 2014


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/121721/#review72646
-----------------------------------------------------------


+1 to me, at this point you know more about this codebase than anybody else here, I think.

I would recommend extending the unit tests so changes are easier to happen further in the future.

- Aleix Pol Gonzalez


On Dec. 28, 2014, 7:29 p.m., Nicolai Hähnle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121721/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2014, 7:29 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> I apologize that this ended up as a rather large chunk of code changes. The essential changes are:
> 
> 1. Use the new IBreakpointController interface, which greatly reduces the coupling between kdevplatform and kdevelop and will allow more changes without worrying about the ABI in the future (the old interface had IBreakpointController implementations poking directly at protected member variables of IBreakpointController).
> 2. We now listen to async notifications where GDB informs us about new, modified, and deleted breakpoints to keep the breakpoint model up-to-date. This should be both more accurate and more efficient (fewer messages sent between KDevelop and GDB) than the old approach.
> 3. Interrupting GDB and (if necessary) restarting the inferior is now handled transparently at the DebugSession level rather than in BreakpointController. This should make it easier to coordinate in the long run, e.g. avoiding unnecessary updates of watched variables when the program is only interrupted temporarily.
> 
> Also of note:
> 1. Use std::function to allow writing lambdas for the command handler callbacks.
> 2. SIGSEGV is no longer treated as stopping the interior. I don't see a reason why it should be treated differently from e.g. SIGBUS. Furthermore, te old code would have gotten terribly confused by a debugged process that successfully handles and recovers from a SIGSEGV.
> 3. Stylistic question: should QSharedPointer or std::shared_ptr be preferred?
> 
> 
> Diffs
> -----
> 
>   debuggers/gdb/breakpointcontroller.h f415922a2cddfadcc56e1fdc712ccc3e09c080b3 
>   debuggers/gdb/breakpointcontroller.cpp 7a274970bb0dcc78aef946be8efe104a3257aab6 
>   debuggers/gdb/debugsession.h 57df80437b1d2eea2a0ba7083af58adf19c6a808 
>   debuggers/gdb/debugsession.cpp 392662440f77a2f1748c8e4c61959bec6a39cb2d 
>   debuggers/gdb/gdb.h 6c7193bde17da933aa4478d25381c3750b60691e 
>   debuggers/gdb/gdb.cpp 2ce000903e226b58fc4b8297dda7e7bfb9bd279a 
>   debuggers/gdb/gdbcommand.h e3cf5877b11649008014d5c9aca8e570017c7550 
>   debuggers/gdb/gdbcommand.cpp 8116c48ca86efabf2540066de4d0133fb232269e 
>   debuggers/gdb/gdbcommandqueue.h eb676a4afd1598a6b5df92af5044d62bb8d3400a 
>   debuggers/gdb/gdbcommandqueue.cpp 3ee29de62727caa4c8d61c598c27ee6770dfe8cc 
>   debuggers/gdb/gdbglobal.h da6fe329e7aba55433c4fbbc54c5ce193100e971 
>   debuggers/gdb/unittests/test_gdb.cpp b9a1a6f41ce83ae3ab523526f69432786617aa24 
> 
> Diff: https://git.reviewboard.kde.org/r/121721/diff/
> 
> 
> Testing
> -------
> 
> unit tests pass; manual testing seems fine
> 
> 
> Thanks,
> 
> Nicolai Hähnle
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20141229/d7f47814/attachment.html>


More information about the KDevelop-devel mailing list