Re: [Kdenlive-svn] [kdenlive] src: Use QPointer [krazy 1/37] by Mikko Rapeli

[laurent M. <mo...@kd...>]

Le lundi 23 juillet 2012 23:29:03 jb a écrit :
> On Monday 23 July 2012 19:22:22 laurent Montel wrote:
> > Use a QPointer<...> but not test it it's not usefull.
> > 
> >  QPointer<ManageCapturesDialog> d = new
> >  ManageCapturesDialog(capturedFiles,
> > 
> > this);
> > if (d->exec() == QDialog::Accepted && d) {
> > ------------------------------------------------^^^
> > 
> >      emit addProjectClipList(d->importFiles());
> > 
> > }
> > 
> > Otherwise it will crash too if dialog was deleted.
> 
> Well, that is an interesting point.
> 
> According to that blog entry: http://blogs.kde.org/node/3919
> It should be safe to use d->exec() on pointers, here is the quote:
> 
> "If [the dialog] was deleted during the exec(), exec() will return
> QDialog::Rejected"
> 
> So did I understand something wrong?
> 
> Thanks anyways for having an eye on our code, and waiting for your answer...
> 
> regards
> jb

Ok was an mistake.
but I saw 2 errors in commit:
http://commits.kde.org/kdenlive/2ef1b6061d86b56b9d17b1de2f60946d39a2cb94
you use without look at pointer:

        QPointer<KFileDialog> fs = new KFileDialog(KUrl(m_projectTitlePath), 
"application/x-kdenlivetitle", this);
        fs->setOperationMode(KFileDialog::Saving);
        fs->setMode(KFile::File);
        fs->setConfirmOverwrite(true);
        fs->setKeepLocation(true);
        fs->exec();
        url = fs->selectedUrl();
        delete fs;


=> test if fs is null or not
http://commits.kde.org/kdenlive/8216494ab1a9f76204d18a61ab1b761516052bbc
int MainWindow::getNewStuff(const QString &configFile)
{
    KNS3::Entry::List entries;
#if KDE_IS_VERSION(4,3,80)
    QPointer<KNS3::DownloadDialog> dialog = new 
KNS3::DownloadDialog(configFile);
    dialog->exec();
    entries = dialog->changedEntries();
    foreach(const KNS3::Entry & entry, entries) {
        if (entry.status() == KNS3::Entry::Installed)
            kDebug() << "// Installed files: " << entry.installedFiles();
    }
    delete dialog;

=> don't test after the exec.

Same here : 
http://commits.kde.org/kdenlive/de5b5ab3701df32ff28275cb1f1f2bf55bb5f493
    QPointer<KFileDialog> fs = new KFileDialog(KUrl(), "image/png", this);
    fs->setOperationMode(KFileDialog::Saving);
    fs->setMode(KFile::File);
    fs->setConfirmOverwrite(true);
    fs->setKeepLocation(true);
    fs->exec();
    QString path = fs->selectedFile();
    delete fs;


Regards




-- 
Laurent Montel | lau...@kd... | KDE/Qt Senior Software Engineer
KDAB (France) S.A.S., a KDAB Group company
Tel. France +33 (0)4 90 84 08 53, Sweden (HQ) +46-563-540090
KDAB - Qt Experts - Platform-independent software solutions