> On March 11, 2013, 9:32 a.m., Giorgos Tsiapaliokas wrote: > > plasmate/mainwindow.cpp, line 858 > > <http://git.reviewboard.kde.org/r/109339/diff/1/?file=117777#file117777line858> > > > > it should be > > > > delete m_konsoleWidget; > > m_konsoleWidget = 0; > > > > after the delete the pointer is dangling, > > of course since createKonsoleFor returns > > either 0 or a valid pointer, we don't have an issue, *but* if in the > > future the code changes we might have a dangling pointer and this isn't > > good :) > > > > > > > > Tianyu Zhu wrote: > I'm simply following the same pattern as the lines of code after it that > initialize m_previwerwidget: > > // initialize previewer > delete m_previewerWidget; > m_previewerWidget = createPreviewerFor(previewerType); > > If we want to fix that the pointer is not being set to 0, then I believe > we should make a earnest effort to find and fix all these cases. > > Alternatively, we can use some sort of smart pointer with unique > ownership semantics that will automatically delete and nullify the pointer if > it is being reset. > > Either way, I believe that change should happen in another patch, since > it is a separate problem from the bug being fixed here. > > Tianyu Zhu wrote: > I would just like to point out that QScopedPointer is a smart pointer > meeting these requirements.
Yes QScopedPointer makes sense. Also QScopedPointer can be used for other private members such as m_previewerWidget(of course this is another patch set). - Giorgos ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/109339/#review28947 ----------------------------------------------------------- On March 7, 2013, 2:10 p.m., Tianyu Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/109339/ > ----------------------------------------------------------- > > (Updated March 7, 2013, 2:10 p.m.) > > > Review request for Plasma. > > > Description > ------- > > If the old konsoleWidget is not deleted when loading a new project, it shows > up along with a new one in the project view. > This also fixes a memory leak. > > > This addresses bug https://bugs.kde.org/show_bug.cgi?id=316276. > > http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=316276 > > > Diffs > ----- > > plasmate/mainwindow.cpp 13234ec > > Diff: http://git.reviewboard.kde.org/r/109339/diff/ > > > Testing > ------- > > It works on my machine: Thinkpad T520, Kubuntu 12.10, KDE 4.10.1 > > > Thanks, > > Tianyu Zhu > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
