[kstars] kstars: Fix crash in observing list whenever catalog configuration was changed.

Akarsh Simha akarsh at kde.org
Fri Oct 14 10:27:43 UTC 2016


Git commit f11601a3f2a7dd99f1fba49425372baf8cc462fc by Akarsh Simha.
Committed on 14/10/2016 at 10:28.
Pushed by asimha into branch 'master'.

Fix crash in observing list whenever catalog configuration was changed.

a.k.a. the "Wishlist-Killer" bug.

A very frustrating crash would ensue whenever the catalog
configuration options were changed (eg: adding a new custom catalog)
and an object was added to the observing wishlist. Before a recent
commit, the wishlist would be truncated causing much frustration to
the user.

The crash occurred because the SkyObjects are de-allocated and
re-allocated by SkyMapComposite::reloadDeepSky() every time the
catalog configuration objects have changed. Since the ObservingList
maintained a QList<SkyObject *> pointers to these SkyObjects, we were
often querying dangling pointers, leading to crashes. So addition of
the object to the wishlist, although did trigger the bug, was not the
root cause.

This change fixes the bug (hopefully without inducing any
others!). The changes are:

1. Create a clone of the SkyObject in ObservingList

   This makes add / remove more complicated as they look-up the given
   object by name, but solves the crash because we have a local copy
   of the SkyObject even if the CatalogComponent's copy is deleted.

2. Use QSharedPointer<SkyObject> instead of SkyObject *

   This makes sure that we don't re-create the same object for session
   plan / wish list. Besides, it is much more cleaner, and takes care
   of deleting itself etc.

Extensive testing is appreciated.

And once again, happy 20th Birthday KDE!

BUG: 345348
CCMAIL: kstars-devel at kde.org

M  +2    -2    kstars/kstarsdbus.cpp
M  +18   -19   kstars/oal/execute.cpp
M  +0    -7    kstars/oal/execute.h
M  +2    -1    kstars/oal/log.cpp
M  +9    -4    kstars/skycomponents/skymapcomposite.cpp
M  +84   -63   kstars/tools/observinglist.cpp
M  +13   -11   kstars/tools/observinglist.h

http://commits.kde.org/kstars/f11601a3f2a7dd99f1fba49425372baf8cc462fc

diff --git a/kstars/kstarsdbus.cpp b/kstars/kstarsdbus.cpp
index de7e89b..7f7e58d 100644
--- a/kstars/kstarsdbus.cpp
+++ b/kstars/kstarsdbus.cpp
@@ -633,7 +633,7 @@ void KStars::renderEyepieceView( const QString &objectName, const QString &destP
 }
 QString KStars::getObservingWishListObjectNames() {
     QString output;
-    foreach( const SkyObject *object,  KStarsData::Instance()->observingList()->obsList() ) {
+    foreach( QSharedPointer<SkyObject> object,  KStarsData::Instance()->observingList()->obsList() ) {
         output.append( object->name() + '\n' );
     }
     return output;
@@ -641,7 +641,7 @@ QString KStars::getObservingWishListObjectNames() {
 
 QString KStars::getObservingSessionPlanObjectNames() {
     QString output;
-    foreach( const SkyObject *object,  KStarsData::Instance()->observingList()->sessionList() ) {
+    foreach( QSharedPointer<SkyObject> object,  KStarsData::Instance()->observingList()->sessionList() ) {
         output.append( object->name() + '\n' );
     }
     return output;
diff --git a/kstars/oal/execute.cpp b/kstars/oal/execute.cpp
index 752628d..a67f895 100644
--- a/kstars/oal/execute.cpp
+++ b/kstars/oal/execute.cpp
@@ -218,8 +218,8 @@ void Execute::slotLocation() {
 void Execute::loadTargets() {
     ui.Target->clear();
     sortTargetList();
-    foreach( SkyObject *o, KStarsData::Instance()->observingList()->sessionList() ) {
-        ui.Target->addItem( getObjectName(o, false) );
+    foreach( QSharedPointer<SkyObject> o, KStarsData::Instance()->observingList()->sessionList() ) {
+        ui.Target->addItem( getObjectName(o.data(), false) );
     }
 }
 
@@ -245,23 +245,22 @@ void Execute::loadObservers() {
 }
 
 void Execute::sortTargetList() {
-    qSort( KStarsData::Instance()->observingList()->sessionList().begin(), KStarsData::Instance()->observingList()->sessionList().end(), Execute::timeLessThan );
-}
-
-bool Execute::timeLessThan ( SkyObject *o1, SkyObject *o2 )
-{
-    QTime t1 = KStarsData::Instance()->observingList()->scheduledTime( o1 );
-    QTime t2 = KStarsData::Instance()->observingList()->scheduledTime( o2 );
-
-    if( t1 < QTime(12,0,0) )
-        t1.setHMS( t1.hour()+12, t1.minute(), t1.second() );
-    else
-        t1.setHMS( t1.hour()-12, t1.minute(), t1.second() );
-    if( t2 < QTime(12,0,0) )
-        t2.setHMS( t2.hour()+12, t2.minute(), t2.second() );
-    else
-        t2.setHMS( t2.hour()-12, t2.minute(), t2.second() );
-    return ( t1 < t2 ) ;
+    auto timeLessThan =  []( QSharedPointer<SkyObject> o1, QSharedPointer<SkyObject> o2 ) {
+        QTime t1 = KStarsData::Instance()->observingList()->scheduledTime( o1.data() );
+        QTime t2 = KStarsData::Instance()->observingList()->scheduledTime( o2.data() );
+
+        if( t1 < QTime(12,0,0) )
+            t1.setHMS( t1.hour()+12, t1.minute(), t1.second() );
+        else
+            t1.setHMS( t1.hour()-12, t1.minute(), t1.second() );
+        if( t2 < QTime(12,0,0) )
+            t2.setHMS( t2.hour()+12, t2.minute(), t2.second() );
+        else
+            t2.setHMS( t2.hour()-12, t2.minute(), t2.second() );
+        return ( t1 < t2 ) ;
+    };
+
+    qSort( KStarsData::Instance()->observingList()->sessionList().begin(), KStarsData::Instance()->observingList()->sessionList().end(), timeLessThan );
 }
 
 void Execute::addTargetNotes() {
diff --git a/kstars/oal/execute.h b/kstars/oal/execute.h
index 5f89192..ffc068b 100644
--- a/kstars/oal/execute.h
+++ b/kstars/oal/execute.h
@@ -89,13 +89,6 @@ Q_OBJECT
          */
         void sortTargetList();
 
-        /** @short Custom LessThan function for the sort by time
-         * This compares the times of the two skyobjects
-         * using an edited QTime as next day 01:00 should
-         * come later than previous night 23:00
-         */
-        static bool timeLessThan( SkyObject *, SkyObject * );
-
         /** @short set the currentTarget when the user selection
          * is changed in the target combo box
          */
diff --git a/kstars/oal/log.cpp b/kstars/oal/log.cpp
index b1c1e2c..1b35952 100644
--- a/kstars/oal/log.cpp
+++ b/kstars/oal/log.cpp
@@ -18,6 +18,7 @@
 
 #include "oal/log.h"
 #include "kstars.h"
+#include "ksutils.h"
 #include "kstarsdata.h"
 #include "skyobjects/skyobject.h"
 #include "skyobjects/starobject.h"
@@ -30,7 +31,7 @@
 void OAL::Log::writeBegin()
 {
     output = "";
-    m_targetList = KStarsData::Instance()->observingList()->sessionList();
+    m_targetList = KSUtils::makeVanillaPointerList( KStarsData::Instance()->observingList()->sessionList() );
     writer = new QXmlStreamWriter(&output);
     writer->setAutoFormatting( true );
     writer->writeStartDocument();
diff --git a/kstars/skycomponents/skymapcomposite.cpp b/kstars/skycomponents/skymapcomposite.cpp
index 3b2395a..a798778 100644
--- a/kstars/skycomponents/skymapcomposite.cpp
+++ b/kstars/skycomponents/skymapcomposite.cpp
@@ -24,6 +24,7 @@
 #include "kstarsdata.h"
 #ifndef KSTARS_LITE
 #include "skymap.h"
+#include "ksutils.h"
 #endif
 #include "skyobjects/starobject.h"
 #include "skyobjects/deepskyobject.h"
@@ -287,10 +288,14 @@ void SkyMapComposite::draw( SkyPainter *skyp )
     // map->infoBoxes()->reserveBoxes( psky );
 
     if( KStars::Instance() ) {
-        const QList<SkyObject*> obsList = KStarsData::Instance()->observingList()->sessionList();
+        auto& obsList = KStarsData::Instance()->observingList()->sessionList();
         if( Options::obsListText() )
-            foreach( SkyObject* obj, obsList ) {
-                SkyLabeler::AddLabel( obj, SkyLabeler::RUDE_LABEL );
+            foreach( QSharedPointer<SkyObject> obj_clone, obsList ) {
+                // Find the "original" obj
+                SkyObject *o = findByName( obj_clone->name() ); // FIXME: This is sloww.... and can also fail!!!
+                if ( !o )
+                    continue;
+                SkyLabeler::AddLabel( o, SkyLabeler::RUDE_LABEL );
             }
     }
 
@@ -340,7 +345,7 @@ void SkyMapComposite::draw( SkyPainter *skyp )
 
     m_ObservingList->pen = QPen( QColor(data->colorScheme()->colorNamed( "ObsListColor" )), 1. );
     if( KStars::Instance() && !m_ObservingList->list )
-        m_ObservingList->list = &KStarsData::Instance()->observingList()->sessionList();
+        m_ObservingList->list = new SkyObjectList( KSUtils::makeVanillaPointerList( KStarsData::Instance()->observingList()->sessionList() ) ); // Make sure we never delete the pointers in m_ObservingList->list!
     if( m_ObservingList )
         m_ObservingList->draw( skyp );
 
diff --git a/kstars/tools/observinglist.cpp b/kstars/tools/observinglist.cpp
index 8001a7e..a126b0b 100644
--- a/kstars/tools/observinglist.cpp
+++ b/kstars/tools/observinglist.cpp
@@ -290,17 +290,17 @@ ObservingList::~ObservingList()
 
 //SLOTS
 
-void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
+void ObservingList::slotAddObject( const SkyObject *_obj, bool session, bool update ) {
     bool addToWishList=true;
-    if( ! obj )
-        obj = SkyMap::Instance()->clickedObject(); // Eh? Why? Weird default behavior.
+    if( ! _obj )
+        _obj = SkyMap::Instance()->clickedObject(); // Eh? Why? Weird default behavior.
 
-    if ( !obj ) {
+    if ( !_obj ) {
         qWarning() << "Trying to add null object to observing list! Ignoring.";
         return;
     }
 
-    QString finalObjectName = getObjectName(obj);
+    QString finalObjectName = getObjectName( _obj );
 
     if (finalObjectName.isEmpty())
     {
@@ -309,19 +309,27 @@ void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
     }
 
     //First, make sure object is not already in the list
-    if ( obsList().contains( obj ) ) {
+    QSharedPointer<SkyObject> obj = findObject( _obj );
+    if ( obj ) {
         addToWishList = false;
         if( ! session ) {
-            KStars::Instance()->statusBar()->showMessage( i18n( "%1 is already in your wishlist.", finalObjectName ), 0 );
+            KStars::Instance()->statusBar()->showMessage( i18n( "%1 is already in your wishlist.", finalObjectName ), 0 ); // FIXME: This message is too inconspicuous if using the Find dialog to add
             return;
         }
     }
+    else {
+        assert( !findObject( _obj, true ) );
+        qDebug() << "Cloned object " << finalObjectName << " to add to observing list.";
+        obj = QSharedPointer<SkyObject>( _obj->clone() ); // Use a clone in case the original SkyObject is deleted due to change in catalog configuration.
+    }
+
 
     if ( session && sessionList().contains( obj ) ) {
         KStars::Instance()->statusBar()->showMessage( i18n( "%1 is already in the session plan.", finalObjectName ), 0 );
         return;
     }
 
+
     // JM: If we are loading observing list from disk, solar system objects magnitudes are not calculated until later
     // Therefore, we manual invoke updateCoords to force computation of magnitude.
     if ( (obj->type() == SkyObject::COMET || obj->type() == SkyObject::ASTEROID || obj->type() == SkyObject::MOON ||
@@ -347,10 +355,10 @@ void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
     };
 
     // Fill itemlist with items that are common to both wishlist additions and session plan additions
-    auto populateItemList = [ &getItemWithUserRole, &itemList, &finalObjectName, &obj, &p, &smag ]() {
+    auto populateItemList = [ &getItemWithUserRole, &itemList, &finalObjectName, obj, &p, &smag ]() {
         itemList.clear();
         QStandardItem *keyItem = getItemWithUserRole( finalObjectName );
-        keyItem->setData( QVariant::fromValue<void *>( static_cast<void *>( obj ) ), Qt::UserRole + 1 );
+        keyItem->setData( QVariant::fromValue<void *>( static_cast<void *>( obj.data() ) ), Qt::UserRole + 1 );
         itemList << keyItem // NOTE: The rest of the methods assume that the SkyObject pointer is available in the first column!
         << getItemWithUserRole( obj->translatedLongName() )
         << getItemWithUserRole( p.ra0().toHMSString() )
@@ -363,7 +371,7 @@ void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
     if( addToWishList ) {
 
         m_WishList.append( obj );
-        m_CurrentObject = obj;
+        m_CurrentObject = obj.data();
 
         //QString ra, dec;
         //ra = "";//p.ra().toHMSString();
@@ -405,7 +413,7 @@ void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
         //}
         // TODO: Change the rest of the parameters to their appropriate datatypes.
         populateItemList();
-        itemList << getItemWithUserRole( KSUtils::constNameToAbbrev( KStarsData::Instance()->skyComposite()->constellationBoundary()->constellationName( obj ) ) )
+        itemList << getItemWithUserRole( KSUtils::constNameToAbbrev( KStarsData::Instance()->skyComposite()->constellationBoundary()->constellationName( obj.data() ) ) )
                  << BestTime
                  << getItemWithUserRole( alt )
                  << getItemWithUserRole( az );
@@ -420,34 +428,37 @@ void ObservingList::slotAddObject( SkyObject *obj, bool session, bool update ) {
     setSaveImagesButton();
 }
 
-void ObservingList::slotRemoveObject( SkyObject *o, bool session, bool update ) {
+void ObservingList::slotRemoveObject( const SkyObject *_o, bool session, bool update ) {
     if( ! update ) { // EH?!
-        if ( ! o )
-            o = SkyMap::Instance()->clickedObject();
+        if ( ! _o )
+            _o = SkyMap::Instance()->clickedObject();
         else if( sessionView ) //else if is needed as clickedObject should not be removed from the session list.
             session = true;
     }
 
-    // Remove from hash
-    ImagePreviewHash.remove(o);
+    // Is the pointer supplied in our own lists?
+    const QList<QSharedPointer<SkyObject>> &list = ( session ? sessionList() : obsList() );
+    QStandardItemModel *currentModel = ( session ? m_SessionModel : m_WishListModel );
 
-    QStandardItemModel *currentModel;
-
-    int k;
-    if (! session) {
-        currentModel = m_WishListModel;
-        k = obsList().indexOf( o );
-    }
-    else {
-        currentModel = m_SessionModel;
-        k = sessionList().indexOf( o );
+    QSharedPointer<SkyObject> o = findObject( _o, session );
+    if ( !o ) {
+        qWarning() << "Object (name: " << getObjectName( o.data() ) << ") supplied to ObservingList::slotRemoveObject() was not found in the " << QString( session ? "session" : "observing" ) << " list!";
+        return;
     }
 
-    if ( o == LogObject ) saveCurrentUserLog();
+    int k = list.indexOf( o );
+    assert( k >= 0 );
+
+    // Remove from hash
+    ImagePreviewHash.remove( o.data() );
+
+    if ( o.data() == LogObject ) saveCurrentUserLog();
+
     //Remove row from the TableView model
+    // FIXME: Is there no faster way?
     for ( int irow = 0; irow < currentModel->rowCount(); ++irow ) {
         QString name = currentModel->item(irow, 0)->text();
-        if ( getObjectName(o) == name ) {
+        if ( getObjectName(o.data()) == name ) {
             currentModel->removeRow(irow);
             break;
         }
@@ -511,7 +522,7 @@ void ObservingList::slotNewSelection() {
     ui->ImagePreview->setCursor( Qt::ArrowCursor );
     QModelIndexList selectedItems;
     QString newName;
-    SkyObject *o;
+    QSharedPointer<SkyObject> o;
     QString labelText;
     ui->DeleteImage->setEnabled( false );
 
@@ -524,7 +535,7 @@ void ObservingList::slotNewSelection() {
         //then break the loop.  Now SessionList.current()
         //points to the new selected object (until now it was the previous object)
         foreach ( o,  getActiveList() ) {
-            if ( getObjectName(o) == newName ) {
+            if ( getObjectName( o.data() ) == newName ) {
                 found = true;
                 break;
             }
@@ -539,11 +550,11 @@ void ObservingList::slotNewSelection() {
         #endif
         if ( found )
         {
-            m_CurrentObject = o;
+            m_CurrentObject = o.data();
             //QPoint pos(0,0);
-            plot( o );
+            plot( o.data() );
             //Change the m_currentImageFileName, DSS/SDSS Url to correspond to the new object
-            setCurrentImage( o );
+            setCurrentImage( o.data() );
             ui->SearchImage->setEnabled( true );
             if ( newName != i18n( "star" ) ) {
                 //Display the current object's user notes in the NotesEdit
@@ -585,11 +596,11 @@ void ObservingList::slotNewSelection() {
                 KSDssImage ksdi( ImagePath );
                 KSDssImage::Metadata md = ksdi.getMetadata();
                 //ui->ImagePreview->showPreview( QUrl::fromLocalFile( ksdi.getFileName() ) );
-                if (ImagePreviewHash.contains(o) == false)
-                    ImagePreviewHash[o] = QPixmap(ksdi.getFileName()).scaledToHeight(ui->ImagePreview->width());
+                if (ImagePreviewHash.contains(o.data()) == false)
+                    ImagePreviewHash[o.data()] = QPixmap(ksdi.getFileName()).scaledToHeight(ui->ImagePreview->width());
 
                 //ui->ImagePreview->setPixmap(QPixmap(ksdi.getFileName()).scaledToHeight(ui->ImagePreview->width()));
-                ui->ImagePreview->setPixmap(ImagePreviewHash[o]);
+                ui->ImagePreview->setPixmap(ImagePreviewHash[o.data()]);
                 if( md.width != 0 ) {// FIXME: Need better test for meta data presence
                     ui->dssMetadataLabel->setText( i18n( "DSS Image metadata: \n Size: %1\' x %2\' \n Photometric band: %3 \n Version: %4",
                                                          QString::number( md.width ), QString::number( md.height ), QString() + md.band, md.version ) );
@@ -604,7 +615,7 @@ void ObservingList::slotNewSelection() {
                 setDefaultImage();
                 ui->dssMetadataLabel->setText( i18n( "No image available. Click on the placeholder image to download one." ) );
             }
-            QString cname = KStarsData::Instance()->skyComposite()->constellationBoundary()->constellationName( o );
+            QString cname = KStarsData::Instance()->skyComposite()->constellationBoundary()->constellationName( o.data() );
             if ( o->type() != SkyObject::CONSTELLATION ) {
                 labelText = "<b>";
                 if ( o->type() == SkyObject::PLANET )
@@ -736,10 +747,9 @@ void ObservingList::slotAddToSession() {
     Q_ASSERT( ! sessionView );
     if ( getSelectedItems().size() ) {
         foreach ( const QModelIndex &i, getSelectedItems() ) {
-            foreach ( SkyObject *o, obsList() )
-                if ( getObjectName(o) == i.data().toString() )
-                    slotAddObject( o, true );
-
+            foreach ( QSharedPointer<SkyObject> o, obsList() )
+                if ( getObjectName( o.data() ) == i.data().toString() )
+                    slotAddObject( o.data(), true ); // FIXME: Would be good to have a wrapper that accepts QSharedPointer<SkyObject>
         }
     }
 }
@@ -833,6 +843,7 @@ void ObservingList::slotOpenList()
         saveCurrentList();//See if the current list needs to be saved before opening the new one
         ui->tabWidget->setCurrentIndex(1);
         slotChangeTab(1);
+
         sessionList().clear();
         TimeHash.clear();
         m_CurrentObject = 0;
@@ -849,7 +860,7 @@ void ObservingList::slotOpenList()
         geo = logObject.geoLocation();
         dt = logObject.dateTime();
         foreach( SkyObject *o, *( logObject.targetList() ) )
-        slotAddObject( o, true );
+            slotAddObject( o, true );
         //Update the location and user set times from file
         slotUpdate();
         //Newly-opened list should not trigger isModified flag
@@ -891,17 +902,17 @@ void ObservingList::slotSaveList() {
 
     QString fileContents;
     QTextStream ostream( &fileContents ); // We first write to a QString to prevent truncating the file in case there is a crash.
-    foreach ( const SkyObject* o, obsList() ) {
+    foreach ( const QSharedPointer<SkyObject> o, obsList() ) {
         if ( !o ) {
             qWarning() << "Null entry in observing wishlist! Skipping!";
             continue;
         }
         if ( o->name() == "star" ) {
             //ostream << o->name() << "  " << o->ra0().Hours() << "  " << o->dec0().Degrees() << endl;
-            ostream << getObjectName(o, false) << endl;
+            ostream << getObjectName(o.data(), false) << endl;
         } else if ( o->type() == SkyObject::STAR ) {
-            Q_ASSERT( dynamic_cast<const StarObject *>( o ) );
-            const StarObject *s = static_cast<const StarObject *>( o );
+            Q_ASSERT( dynamic_cast<const StarObject *>( o.data() ) );
+            const QSharedPointer<StarObject> s = qSharedPointerCast<StarObject>( o );
             if ( s->name() == s->gname() )
                 ostream << s->name2() << endl;
             else
@@ -1150,17 +1161,17 @@ void ObservingList::slotUpdate() {
     dt.setDate( ui->DateEdit->date() );
     ui->avt->removeAllPlotObjects();
     //Creating a copy of the lists, we can't use the original lists as they'll keep getting modified as the loop iterates
-    QList<SkyObject*> _obsList=m_WishList, _SessionList=m_SessionList;
-    foreach ( SkyObject *o, _obsList ) {
+    QList<QSharedPointer<SkyObject>> _obsList=m_WishList, _SessionList=m_SessionList;
+    foreach ( QSharedPointer<SkyObject> o, _obsList ) {
         if( o->name() != "star" ) {
-            slotRemoveObject( o, false, true );
-            slotAddObject( o, false, true );
+            slotRemoveObject( o.data(), false, true );
+            slotAddObject( o.data(), false, true );
         }
     }
-    foreach ( SkyObject *obj, _SessionList ) {
+    foreach ( QSharedPointer<SkyObject> obj, _SessionList ) {
         if( obj->name() != "star" ) {
-            slotRemoveObject( obj, true, true );
-            slotAddObject( obj, true, true );
+            slotRemoveObject( obj.data(), true, true );
+            slotAddObject( obj.data(), true, true );
         }
     }
 }
@@ -1288,15 +1299,15 @@ void ObservingList::slotSaveAllImages() {
     ui->WishListView->clearSelection();
     ui->SessionView->clearSelection();
 
-    foreach( SkyObject *o, getActiveList() ) {
+    foreach( QSharedPointer<SkyObject> o, getActiveList() ) {
         if( !o )
             continue; // FIXME: Why would we have null objects? But appears that we do.
-        setCurrentImage( o );
+        setCurrentImage( o.data() );
         QString img( getCurrentImagePath()  );
         //        QUrl url( ( Options::obsListPreferDSS() ) ? DSSUrl : SDSSUrl ); // FIXME: We have removed SDSS support!
-        QUrl url( KSDssDownloader::getDSSURL( o ) );
+        QUrl url( KSDssDownloader::getDSSURL( o.data() ) );
         if( ! o->isSolarSystem() )//TODO find a way for adding support for solar system images
-            saveImage( url, img, o );
+            saveImage( url, img, o.data() );
     }
 }
 
@@ -1471,17 +1482,17 @@ void ObservingList::slotAddVisibleObj() {
     selectedItems = m_WishListSortModel->mapSelectionToSource( ui->WishListView->selectionModel()->selection() ).indexes();
     if ( selectedItems.size() )
         foreach ( const QModelIndex &i, selectedItems ) {
-            foreach ( SkyObject *o, obsList() )
-                if ( getObjectName(o) == i.data().toString() && w->checkVisibility( o ) )
-                    slotAddObject( o, true );
+            foreach ( QSharedPointer<SkyObject> o, obsList() )
+                if ( getObjectName( o.data() ) == i.data().toString() && w->checkVisibility( o.data() ) )
+                    slotAddObject( o.data(), true ); // FIXME: Better if there is a QSharedPointer override for this, although the check will ensure that we don't duplicate.
         }
     delete w;
 }
 
 SkyObject* ObservingList::findObjectByName( QString name ) {
-    foreach( SkyObject* o, sessionList() )
-        if( getObjectName(o, false) == name )
-            return o;
+    foreach( QSharedPointer<SkyObject> o, sessionList() )
+        if( getObjectName(o.data(), false) == name )
+            return o.data();
     return NULL;
 }
 
@@ -1540,3 +1551,13 @@ void ObservingList::slotUpdateAltitudes() {
     emit m_WishListModel->dataChanged( m_WishListModel->index( 0, m_WishListModel->columnCount() - 1 ),
                                        m_WishListModel->index( m_WishListModel->rowCount() - 1, m_WishListModel->columnCount() - 1 ) );
 }
+
+QSharedPointer<SkyObject> ObservingList::findObject( const SkyObject* o, bool session ) {
+    const QList<QSharedPointer<SkyObject>> &list = ( session ? sessionList() : obsList() );
+    const QString &target = getObjectName( o );
+    foreach( QSharedPointer<SkyObject> obj, list ) {
+        if ( getObjectName( obj.data() ) == target )
+            return obj;
+    }
+    return QSharedPointer<SkyObject>(); // null pointer
+}
diff --git a/kstars/tools/observinglist.h b/kstars/tools/observinglist.h
index 4838816..a7e825b 100644
--- a/kstars/tools/observinglist.h
+++ b/kstars/tools/observinglist.h
@@ -94,22 +94,17 @@ public:
         */
     ~ObservingList();
 
-    /** @return true if the object is in the observing list
-        *@p o pointer to the object to test.
-        */
-    inline bool contains( const SkyObject *o ) { return m_WishList.contains( const_cast<SkyObject*>(o) ); }
-
     /** @return true if the window is in its default "large" state.
         */
     bool isLarge() const { return bIsLarge; }
 
     /** @return reference to the current observing list
         */
-    QList<SkyObject*>& obsList() { return m_WishList; }
+    QList<QSharedPointer<SkyObject>>& obsList() { return m_WishList; }
 
     /** @return reference to the current observing list
         */
-    QList<SkyObject*>& sessionList() { return m_SessionList; }
+    QList<QSharedPointer<SkyObject>>& sessionList() { return m_SessionList; }
 
     /** @return pointer to the currently-selected object in the observing list
         *@note if more than one object is selected, this function returns 0.
@@ -189,13 +184,20 @@ public:
      */
     QString getObjectName(const SkyObject *o, bool translated=true);
 
+    /**
+     * @return true if the selected list contains the given object
+     */
+    inline bool contains( const SkyObject *o, bool session = false ) { return bool( findObject( o, session ) ); }
+
+    QSharedPointer<SkyObject> findObject( const SkyObject *o, bool session = false );
+
 public slots:
     /** @short add a new object to list
         *@p o pointer to the object to add to the list
         *@p session flag toggle adding the object to the session list
         *@p update flag to toggle the call of slotSaveList
         */
-    void slotAddObject( SkyObject *o=NULL, bool session=false, bool update=false );
+    void slotAddObject( const SkyObject *o=NULL, bool session=false, bool update=false );
 
     /** @short Remove skyobjects which are highlighted in the
         *observing list tool from the observing list.
@@ -209,7 +211,7 @@ public slots:
         *@p update flag to toggle the call of slotSaveList
         *Use SkyMap::clickedObject() if o is NULL (default)
         */
-    void slotRemoveObject( SkyObject *o=NULL, bool session=false, bool update=false );
+    void slotRemoveObject( const SkyObject *o=NULL, bool session=false, bool update=false );
 
     /** @short center the selected object in the display
         */
@@ -368,7 +370,7 @@ private:
      * @short Return the active list
      * @return The session list or the wish list depending on which tab is currently being viewed.
      */
-    inline QList<SkyObject *>& getActiveList() { return ( ( sessionView ) ? m_SessionList : m_WishList ); }
+    inline QList<QSharedPointer<SkyObject>>& getActiveList() { return ( ( sessionView ) ? m_SessionList : m_WishList ); }
 
     /**
      * @short Return the active itemmodel
@@ -396,7 +398,7 @@ private:
 
     KSAlmanac *ksal;
     ObservingListUI *ui;
-    QList<SkyObject*> m_WishList, m_SessionList;
+    QList<QSharedPointer<SkyObject>> m_WishList, m_SessionList;
     SkyObject *LogObject, *m_CurrentObject;
     bool isModified, bIsLarge, sessionView, dss, singleSelection, showScope, noSelection;
     QString m_listFileName, m_currentImageFileName, ThumbImage;


More information about the Kstars-devel mailing list