<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/126383/">https://git.reviewboard.kde.org/r/126383/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 16th, 2015, 5:05 p.m. UTC, <b>Christian Ehrlicher</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;">I'm sorry but this is still wrong. Did you read this blog: https://blogs.kde.org/node/3919
You somehow have to notice the the dialog gets deleted during exec(). This is only possible with a QPointer. After exec() when you want to access the pointer, you've to check if the QPointer is null before accesing them:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QPointer<KNS3::DownloadDialog> dialog(new KNS3::DownloadDialog(q));
dialog->exec();
if (dialog)
{
dialog->getValues();
}
delete dialog;</p></pre>
</blockquote>
<p>On December 17th, 2015, 8:30 a.m. UTC, <b>Frederik Schwarzer</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;">No need to be sorry. I put this here to get this kind of feedback. :)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">So, if I get this right (also the content of the page you linked to) your critisism is not against the ScopedPointer itself but against how I used it.
Because QScopedPointer also has the bool overload and can be used in if conditions.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QScopedPointer<KNS3::DownloadDialog> dialog(new KNS3::DownloadDialog(q));
dialog->exec();
if (dialog && !dialog->changedEntries().isEmpty())
{
m_provider->rediscoverThemes();
fillList();
}</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Seems to be alright?</p></pre>
</blockquote>
<p>On December 17th, 2015, 8:43 a.m. UTC, <b>Christian Ehrlicher</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;">Yes, that's correct :)</p></pre>
</blockquote>
<p>On December 17th, 2015, 10:17 a.m. UTC, <b>Christian Ehrlicher</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;">I should have read a little bit more carefully. QScopedPointer is <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">not</em> reset when dialog is deleted inside exec(). So the only way is to use QPointer + delete.</p></pre>
</blockquote>
<p>On December 17th, 2015, 7:22 p.m. UTC, <b>Frederik Schwarzer</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;">Hmm, might you be so kind to point me to where you read that?</p></pre>
</blockquote>
<p>On December 17th, 2015, 11:07 p.m. UTC, <b>Albert Astals Cid</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;">A QScopedPointer is a pointer that deletes itself when the QScopedPointer goes out of scope (see the Qt documentation), similar to what QMutexLocker does, it does not work for tracking the object being deleted elsewhere, do that you need reference tracking (QSharedPointer et al) or in case of QObject base pointers a QPointer</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;">Ok, "tracking the object" helped me scan the QPointer decumentation for the info I lacked. Thanks.</p></pre>
<br />
<p>- Frederik</p>
<br />
<p>On December 17th, 2015, 9:56 a.m. UTC, Frederik Schwarzer 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 Games.</div>
<div>By Frederik Schwarzer.</div>
<p style="color: grey;"><i>Updated Dec. 17, 2015, 9:56 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
libkdegames
</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;">I recently found out about QScopedPointer and thought it might be a good thing to use instead of QPointer in many cases.
So I tested it with two crashy dialogs in libkdegames.
Please comment in case I forgot to think about something.</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;">Used the KNS3 dialog in Kigo.</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>kgthemeselector.cpp <span style="color: grey">(9cd9c61)</span></li>
<li>libkdegamesprivate/kgamethemeselector.cpp <span style="color: grey">(ce3cf1f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/126383/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>