<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 desembre 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 desembre 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 desembre 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 desembre 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 desembre 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>
</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;">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>
<br />
<p>- Albert</p>
<br />
<p>On desembre 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 des. 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>