summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAkarsh Simha <akarsh@kde.org>2016-10-02 12:33:27 (GMT)
committerAkarsh Simha <akarsh@kde.org>2016-10-02 12:35:58 (GMT)
commit0bdaf2dcadcd0b014d2789dfae5481a8a4a9026c (patch)
tree419f399dac74a503daf962334c0a07064170bd0a
parent55a83c05d27c22c68a84b2e217da26c0b50c2b29 (diff)
Prevent unsafe connect by accepting slot in KSDssDownloader constructor
KSDssDownloader commits suicide, so code like: KSDssDownloader *foo = new KSDssDownloader( ... ); connect( foo, ... ); has potential to fail. N.B. The better and more correct fix would be to make the one-shot use-case of KSDssDownloader a static method, just like the one-shot QTimer. It can internally create a class instance and safely handle its destruction.
-rw-r--r--kstars/auxiliary/ksdssdownloader.cpp5
-rw-r--r--kstars/auxiliary/ksdssdownloader.h6
-rw-r--r--kstars/kstarsdbus.cpp7
-rw-r--r--kstars/tools/nameresolver.cpp1
-rw-r--r--kstars/tools/observinglist.cpp4
5 files changed, 16 insertions, 7 deletions
diff --git a/kstars/auxiliary/ksdssdownloader.cpp b/kstars/auxiliary/ksdssdownloader.cpp
index c5f9301..edb53f7 100644
--- a/kstars/auxiliary/ksdssdownloader.cpp
+++ b/kstars/auxiliary/ksdssdownloader.cpp
@@ -38,14 +38,17 @@ KSDssDownloader::KSDssDownloader( QObject *parent ) : QObject( parent ) {
m_VersionPreference << "poss2ukstu_blue" << "poss2ukstu_red" << "poss1_blue" << "poss1_red" << "quickv" << "poss2ukstu_ir";
m_TempFile.open();
}
-KSDssDownloader::KSDssDownloader(const SkyPoint * const p, const QString &destFileName, QObject *parent ) : QObject( parent ) {
+
+KSDssDownloader::KSDssDownloader(const SkyPoint * const p, const QString &destFileName, const std::function<void( bool )> &slotDownloadReady, QObject *parent ) : QObject( parent ) {
// Initialize version preferences. FIXME: This must be made a
// user-changeable option just in case someone likes red
m_VersionPreference << "poss2ukstu_blue" << "poss2ukstu_red" << "poss1_blue" << "poss1_red" << "quickv" << "poss2ukstu_ir";
m_TempFile.open();
+ connect( this, &KSDssDownloader::downloadComplete, slotDownloadReady );
startDownload( p, destFileName );
}
+
QString KSDssDownloader::getDSSURL( const SkyPoint * const p, const QString &version, struct KSDssImage::Metadata *md ) {
const DeepSkyObject *dso = 0;
double height, width;
diff --git a/kstars/auxiliary/ksdssdownloader.h b/kstars/auxiliary/ksdssdownloader.h
index 6045ace..8f8746b 100644
--- a/kstars/auxiliary/ksdssdownloader.h
+++ b/kstars/auxiliary/ksdssdownloader.h
@@ -27,6 +27,8 @@
#include <QStringList>
#include <QTemporaryFile>
+#include <functional>
+
class FileDownloader;
class SkyPoint;
class dms;
@@ -54,12 +56,12 @@ class KSDssDownloader : public QObject {
KSDssDownloader( QObject *parent = 0 );
/**
- * @short Constructor that initiates a "standard" DSS download job, and finally self destructs
+ * @short Constructor that initiates a "standard" DSS download job, calls the downloadReady slot, and finally self destructs
* @note Very important that if you create with this constructor,
* the object will self-destruct. Avoid keeping pointers to it, or
* things may segfault!
*/
- KSDssDownloader( const SkyPoint * const p, const QString &destFileName, QObject *parent = 0 );
+ KSDssDownloader( const SkyPoint * const p, const QString &destFileName, const std::function<void(bool)> &slotDownloadReady, QObject *parent = 0 );
/**
* @short Stateful single-download of a supplied URL. Use when the flexibility is required
diff --git a/kstars/kstarsdbus.cpp b/kstars/kstarsdbus.cpp
index 62f5f3c..de7e89b 100644
--- a/kstars/kstarsdbus.cpp
+++ b/kstars/kstarsdbus.cpp
@@ -604,9 +604,12 @@ void KStars::renderEyepieceView( const QString &objectName, const QString &destP
if( !QFile::exists( imagePath ) ) {
// We must download a DSS image
tempFile.open();
- KSDssDownloader *dler = new KSDssDownloader( target, tempFile.fileName() );
QEventLoop loop;
- connect( dler, SIGNAL( downloadComplete( bool ) ), &loop, SLOT( quit() ) );
+ std::function<void( bool )> slot = [ &loop ]( bool unused ) {
+ Q_UNUSED( unused );
+ loop.quit();
+ };
+ new KSDssDownloader( target, tempFile.fileName(), slot, this );
qDebug() << "DSS download requested. Waiting for download to complete...";
loop.exec(); // wait for download to complete
imagePath = tempFile.fileName();
diff --git a/kstars/tools/nameresolver.cpp b/kstars/tools/nameresolver.cpp
index c8ada23..85f4cb8 100644
--- a/kstars/tools/nameresolver.cpp
+++ b/kstars/tools/nameresolver.cpp
@@ -69,6 +69,7 @@ bool NameResolver::NameResolverInternals::sesameResolver( class CatalogEntryData
QNetworkAccessManager manager;
QNetworkReply *response = manager.get( QNetworkRequest( resolverUrl ) );
+ Q_ASSERT( response );
// Wait synchronously
QEventLoop event;
diff --git a/kstars/tools/observinglist.cpp b/kstars/tools/observinglist.cpp
index ddfc13b..b9e9613 100644
--- a/kstars/tools/observinglist.cpp
+++ b/kstars/tools/observinglist.cpp
@@ -1204,8 +1204,8 @@ void ObservingList::slotGetImage( bool _dss, const SkyObject *o ) {
//QUrl url;
dss = true;
qWarning() << "FIXME: Removed support for SDSS. Until reintroduction, we will supply a DSS image";
- KSDssDownloader *dler = new KSDssDownloader( o, currentImagePath );
- connect( dler, SIGNAL( downloadComplete( bool ) ), SLOT( downloadReady( bool ) ) );
+ std::function<void( bool )> slot = std::bind( &ObservingList::downloadReady, this, std::placeholders::_1 );
+ KSDssDownloader *dler = new KSDssDownloader( o, currentImagePath, slot, this );
}
void ObservingList::downloadReady( bool success ) {