<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122232/">https://git.reviewboard.kde.org/r/122232/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 27th, 2015, 6:31 a.m. UTC, <b>Matthew Dawson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Sorry, I just discovered an issue with this change as is. If the global object is created before KConfig's Global object, a seg fault develops. In this case, KConfig's global is created first avoiding the pain. However, commenting out the noted line causes a seg fault in the tests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't think we can assume people will know to create appropriate KConfig objects, so I'd like to avoid relying on this in the test to pass. I'm not sure how we can force the creation of the global ahead of time. My best thought would be some sort of global pointer to the QStringList, combined with some atomic pointer operations to create it. Thoughts?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Otherwise, everything looked go to me.</p></pre>
</blockquote>
<p>On January 27th, 2015, 8:04 a.m. UTC, <b>David Faure</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The problem is not the order of construction in itself (construction is on demand), but the order of destruction (which is reverse to the order of construction, at least on linux). I know that it crashes without this line, but it does that in kdelibs4 too:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Fatal Error: Accessed global static 'KGlobalPrivate *globalData()' after destruction. Defined at kde/kdelibs/kdecore/kernel/kglobal.cpp:128</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Therefore I don't consider this a real problem. This crash or assert comes from relying on the order of destruction, which one is not supposed to do. In practice most apps will use KConfig soon after creating the app anyway, for their mainwindow, which is why we didn't have a big problem with this.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">If we really want to fix this, however, Q_CONSTRUCTOR_FUNCTION(someFunc) (in ksharedconfig.cpp) comes to mind. That would allow to automate exactly what the line in the test does, calling openConfig right after the qapp is created. I'm just not sure if this is a good idea ;)
Hmm, I'm slowly warming up to the idea, seeing that it doesn't actually create a file if the app doesn't end up using KConfig at all (but just links to the lib indirectly). Let me try it out.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I mean Q_COREAPP_STARTUP_FUNCTION.
Q_CONSTRUCTOR_FUNCTION is too early, no qApp yet.</p></pre>
<br />
<p>- David</p>
<br />
<p>On January 26th, 2015, 8:11 a.m. UTC, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and Matthew Dawson.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated Jan. 26, 2015, 8:11 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kconfig
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">kconfig_in_global_object.cpp comes from kdelibs4support
(after porting to Q_GLOBAL_STATIC)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">ksharedconfig_in_global_object.cpp is new (but works with kdelibs4)
and reproduces Albert's KgDifficulty testcase.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Both tests pass and the QCoreApplication::arguments warning (because called after qApp is destroyed) is gone.</p></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>autotests/CMakeLists.txt <span style="color: grey">(b91f754b705fc87bb8b729bea72fbb5f7d427ace)</span></li>
<li>autotests/kconfig_in_global_object.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>autotests/ksharedconfig_in_global_object.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>src/core/kconfig.cpp <span style="color: grey">(782e9714521234a3e3d8f3a788967e5c9a40f38a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122232/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>