Review Request 121442: gdb: Refactoring of GDB/MI and state tracking (work in progress)

[Milian Wolff]

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

Ship it!


Just some minor nitpicks from my side, please fix it and go ahead pushing this when the tests all pass.


debuggers/gdb/debugsession.h
<https://git.reviewboard.kde.org/r/121442/#comment50070>

    please don't inline stuff, or if you do, then still use the "verbose" style:
    
        void setTesting(bool testing)
        {
            m_testing = testing;
        }



debuggers/gdb/gdb.h
<https://git.reviewboard.kde.org/r/121442/#comment50071>

    style: put & next to typename



debuggers/gdb/gdb.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50072>

    see above



debuggers/gdb/gdb.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50073>

    remove dead code



debuggers/gdb/mi/gdbmi.h
<https://git.reviewboard.kde.org/r/121442/#comment50074>

    & next to typename, and you don't need the `_` suffix, also below



debuggers/gdb/mi/miparser.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50077>

    is the explicit move really required?



debuggers/gdb/mi/miparser.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50076>

    return {};



debuggers/gdb/mi/miparser.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50075>

    explicit move is not required here, or is it?



debuggers/gdb/unittests/test_gdb.cpp
<https://git.reviewboard.kde.org/r/121442/#comment50078>

    this is also done elsewhere, right? can you factor it out into a function?


- Milian Wolff


On Dec. 11, 2014, 2:54 p.m., Nicolai Hähnle wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/121442/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2014, 2:54 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Repository: kdevelop
> 
> 
> Description
> -------
> 
> Please excuse what is probably a bit of an abuse of the system. I wanted to use KDevelop in remote debugging and ran into trouble because it gets horribly confused when user commands (including from a GDB setup script as specified in the Launch Configuration) modify certain kinds of state. I started working on this and things kind of escalated, so I wanted to put something out there ASAP to let you know that I'm working on this and to get feedback on the direction this is going.
> 
> The overall direction of those changes is to modify the gdb plugin such that it picks up state changes (running state, breakpoints, etc.) based on the async notifications rather than based on the command it sends itself. This is crucial for handling correctly the case where the user enters a command such as "continue" directly in the GDB view, or (more importantly for my use case) to handle at all the case where I set up a GDB script to attach to a remote target.
> 
> The first milestone I set for myself is being able to do *any* remote debugging in my particular use case at all, while keeping the unit tests working correctly. Longer term, I hope to be able to fix the breakpoint handling so that it picks up breakpoints set outside the control of KDevelop, and other similar issues as they come up.
> 
> This is a series of patches, the headlines of which are currently:
> 
> gdb: remove unused QueueWhileInterrupted mode from the command queue
> gdb: do not use addCommandToFront in DisassembleWidget
> gdb: DebugSession: add a testing mode for unit tests
> gdb: match structure more closely to the GDB/MI documentation and use cmd tokens
> gdb: always process async notifications from GDB
> gdb: remove magic constants in GDBMI::StreamRecord
> gdb: remove flaky DebugSession::parseStreamRecord
> gdb: add braces around multi-line if statement
> gdb: examineCoreFile: update IDebugSession::state() to match unit tests
> gdb: update session state more robustly based on async notifications
> 
> 
> Diffs
> -----
> 
>   debuggers/gdb/debugsession.h dbe06c5 
>   debuggers/gdb/debugsession.cpp 63f9a58 
>   debuggers/gdb/disassemblewidget.cpp 67024cf 
>   debuggers/gdb/gdb.h 31688cd 
>   debuggers/gdb/gdb.cpp 4dc7b5d 
>   debuggers/gdb/gdbcommand.h fb23c47 
>   debuggers/gdb/gdbcommand.cpp 67abbd8 
>   debuggers/gdb/breakpointcontroller.h 1b8ae0e 
>   debuggers/gdb/breakpointcontroller.cpp d5ba5dd 
>   debuggers/gdb/debuggerplugin.h 4c6bb07 
>   debuggers/gdb/DESIGN.txt ddefb70 
>   debuggers/gdb/gdbcommandqueue.h 7d82f45 
>   debuggers/gdb/gdbcommandqueue.cpp 801bc41 
>   debuggers/gdb/gdbglobal.h ef31c02 
>   debuggers/gdb/mi/gdbmi.h d5e92b8 
>   debuggers/gdb/mi/miparser.h 7096af0 
>   debuggers/gdb/mi/miparser.cpp 123c97e 
>   debuggers/gdb/unittests/test_gdb.h b9a961d 
>   debuggers/gdb/unittests/test_gdb.cpp 03a1203 
>   debuggers/gdb/variablecontroller.h f5ac8c1 
>   debuggers/gdb/variablecontroller.cpp 5eb9908 
> 
> Diff: https://git.reviewboard.kde.org/r/121442/diff/
> 
> 
> Testing
> -------
> 
> unit tests; debugging KDevelop with itself and some remote debugging
> 
> 
> Thanks,
> 
> Nicolai Hähnle
> 
>

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