<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113714/">http://git.reviewboard.kde.org/r/113714/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
<br />
<p>- David Faure</p>
<br />
<p>On November 7th, 2013, 7:52 p.m. UTC, Dan Vrátil wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By Dan Vrátil.</div>
<p style="color: grey;"><i>Updated Nov. 7, 2013, 7:52 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/xmlgui/kxmlguiwindow.cpp <span style="color: grey">(aa4a067)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113714/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>