-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113714/#review43251
-----------------------------------------------------------

Ship it!


The analysis and patch seem correct.

I was initially surprised, because I thought one could have inserted other 
clients into the xmlguifactory (e.g. plugins) before calling createGUI(), but 
clearly that wouldn't work, given the code that deletes all toolbars *and* 
clears the menubar. So we can safely ignore this case, it's not supported (when 
KXmlGuiWindow::createGUI is called).
I think it's supported when using KParts::MainWindow::createGUI, but that code 
does NOT call KXmlGuiWindow::createGUI, so everything is fine.

- David Faure


On Nov. 7, 2013, 7:52 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113714/
> -----------------------------------------------------------
> 
> (Updated Nov. 7, 2013, 7:52 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> Call KXMLGUIFactory::reset() before calling KXMLGUIFactory::addClient(this) 
> in KXMLGuiWindow::createGUI(), otherwise a crash can occur:
> 
> createGUI() calls qDeleteAll(toolbars()) before it starts building the GUI, 
> which can leave KXMLGUIFactory::Private::m_rootNode with ContainerNodes 
> internally pointing to, now an invalid, pointer to KToolBar(s). This won't 
> obviously happen when createGUI() is only called once (which is what most 
> apps do), but it can happen when it is called multiple times (reinstalling UI 
> on tab change for example). Leaving m_rootNode with invalid ContainerNodes 
> causes KXMLGUIBuilder to crash, because it tries to add items into an invalid 
> KToolBar.
> 
> Calling KXMLGUIFactory::reset() first will make sure the m_rootNode is 
> purged, all invalid pointers are removed and KXMLGUIBuilder instantiates a 
> new KToolBar before inserting items.
> 
> 
> Diffs
> -----
> 
>   kdeui/xmlgui/kxmlguiwindow.cpp aa4a067 
> 
> Diff: http://git.reviewboard.kde.org/r/113714/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

Reply via email to