[Kde-games-devel] Review Request 126383: Use of QScopedPointer for avoiding crashy dialogs.

[Frederik Schwarzer]

> On Dec. 16, 2015, 5:05 p.m., Christian Ehrlicher wrote:
> > 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:
> > 
> > QPointer<KNS3::DownloadDialog> dialog(new KNS3::DownloadDialog(q));
> > dialog->exec();
> > if (dialog)
> > {
> >    dialog->getValues();
> > }
> > delete dialog;
> 
> Frederik Schwarzer wrote:
>     No need to be sorry. I put this here to get this kind of feedback. :)
>     
>     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.
>     
>     QScopedPointer<KNS3::DownloadDialog> dialog(new KNS3::DownloadDialog(q));
>     dialog->exec();
>     if (dialog && !dialog->changedEntries().isEmpty())
>     {
>         m_provider->rediscoverThemes();
>         fillList();
>     }
>     
>     Seems to be alright?
> 
> Christian Ehrlicher wrote:
>     Yes, that's correct :)
> 
> Christian Ehrlicher wrote:
>     I should have read a little bit more carefully. QScopedPointer is *not* reset when dialog is deleted inside exec(). So the only way is to use QPointer + delete.
> 
> Frederik Schwarzer wrote:
>     Hmm, might you be so kind to point me to where you read that?
> 
> Albert Astals Cid wrote:
>     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

Ok, "tracking the object" helped me scan the QPointer decumentation for the info I lacked. Thanks.


- Frederik


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126383/#review89612
-----------------------------------------------------------


On Dec. 17, 2015, 9:56 a.m., Frederik Schwarzer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126383/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 9:56 a.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Repository: libkdegames
> 
> 
> Description
> -------
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   kgthemeselector.cpp 9cd9c61 
>   libkdegamesprivate/kgamethemeselector.cpp ce3cf1f 
> 
> Diff: https://git.reviewboard.kde.org/r/126383/diff/
> 
> 
> Testing
> -------
> 
> Used the KNS3 dialog in Kigo.
> 
> 
> Thanks,
> 
> Frederik Schwarzer
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20151218/d8aa30d7/attachment.html>