[Differential] [Requested Changes To] D4232: Bring back memory view

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  some more nitpicks (sorry!), but at least fixing the memory leaks is a must

INLINE COMMENTS

> CMakeLists.txt:54
>  
> +configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/config-kdev-debuggers.h.cmake
> +                ${CMAKE_CURRENT_BINARY_DIR}/config-kdev-debuggers.h )

kdev-debuggers? config-gdb-plugin.h is more like it

> config-kdev-debuggers.h.cmake:1
> +/* This file is generated from template using CMAKE */
> +

please still add a proper copyright header

> config-kdev-debuggers.h.cmake:3
> +
> +#define KDEV_WITH_MEMVIEW @KDEV_WITH_MEMVIEW@

use #cmakedefine01

> memviewdlg.cpp:130
>  
> -    connect(KDevelop::ICore::self()->debugController(),
> -            SIGNAL(currentSessionChanged(KDevelop::IDebugSession*)),
> -            SLOT(currentSessionChanged(KDevelop::IDebugSession*)));
> +    KDevelop::IDebugController* pDC = KDevelop::ICore::self()->debugController();
> +    Q_ASSERT(pDC);

`auto debugController = ...`

> memviewdlg.cpp:133
> +
> +    connect(pDC,
> +            &KDevelop::IDebugController::currentSessionChanged,

style:

  connect(debugController, &...,
          this, &...);

> memviewdlg.cpp:158
>  
> -    khexedit2_widget = KHE::createBytesEditWidget(this);
> -    if (!khexedit2_widget)
> -    {
> -        QTextEdit* edit = new QTextEdit(this);
> -        l->addWidget(edit);
> +    m_memViewModel = new Okteta::ByteArrayModel;
> +    m_memViewView = new Okteta::ByteArrayColumnView(this);

missing parent

> memviewdlg.cpp:255
>  
> -    KHE::BytesEditInterface* bytesEditor = KHE::bytesEditInterface(khexedit2_widget);
> -    bytesEditor->setData(this->data_, 0);
> -
> -    delete[] this->data_;
> -    this->data_ = new char[amount_];
> +    delete[] this->m_memData;
> +    this->m_memData = new char[m_memAmount];

remove this->

> memviewdlg.h:116
> +        QString m_memStartStr, m_memAmountStr;
> +        char* m_memData;
> +        int m_debuggerState;

I know, old code, but this is leaked. I cannot find a dtor that deletes it at least. Use a proper container class, like QByteArray or {Q,std::}vector<char>

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D4232

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: volden, #kdevelop, mwolff
Cc: mwolff, kossebau, kdevelop-devel, Pilzschaf, akshaydeo, surgenight, arrowdodger
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20170129/e08c6ef4/attachment-0001.html>