[patch] forward null check and then dereference
Erik Hovland
erik at hovland.org
Fri Dec 1 22:47:00 UTC 2006
This patch addresses the possibility of dereferencing a pointer that
might be null.
Annotation included in the patch.
Thanks
E
--
Erik Hovland
mail: erik AT hovland DOT org
web: http://hovland.org/
PGP/GPG public key available on request
-------------- next part --------------
Fixes any forward nulls
Forward nulls are any pointers where the pointer is checked for null at
some spot in the code and later the pointer is dereferenced without checking
to see if it is null. The check is required because if it is null it leads
to segfaults.
o src/app.cpp
Check to make sure that m_playlistWindow is not null before doing the show()
o src/collectionbrowser.cpp
No check for the passed in list view item. Check added.
o dynamic_cast in magnatunebrowser et al.
Code how you like. But my experience has been that if there are any more
then one or two dynamic_casts of any object then chances are there should
be a refactoring. But I digress, and that is only a tule of thumb. Anyhow
dynamic_cast can return 0. So it should be checked before dereferencing
the result. This patch does that.
o src/playlistbrowser.cpp
A parent poiner might be null. This patch papers over what I consider to
be strange code. But I couldn't figure out what it was trying to do, so
I just added the check instead of rewriting it to be prettier.
o src/playlistwindow.cpp
Find the lastitem is a little kludgy and can end up in it being null and
then dereferenced. Rework it to be more readable and safe from dereference.
From: Erik Hovland <erik at hovland.org>
Signed-off-by: Erik Hovland <erik at hovland.org>
---
src/app.cpp | 5 +++--
src/collectionbrowser.cpp | 3 +++
src/magnatunebrowser/magnatunebrowser.cpp | 8 ++++++++
src/magnatunebrowser/magnatunelistview.cpp | 4 ++++
src/mediabrowser.cpp | 2 +-
src/mediadevice/daap/daapclient.cpp | 4 ++++
src/playlistbrowser.cpp | 8 +++++---
src/playlistwindow.cpp | 4 +++-
8 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/src/app.cpp b/src/app.cpp
index a607bee..3f866fa 100644
--- a/src/app.cpp
+++ b/src/app.cpp
@@ -615,9 +615,10 @@ void App::applySettings( bool firstTime
//on startup we need to show the window, but only if it wasn't hidden on exit
//and always if the trayicon isn't showing
- if( firstTime && !Amarok::config()->readBoolEntry( "HiddenOnExit", false ) || !AmarokConfig::showTrayIcon() )
+ QWidget* main_window = mainWindow();
+ if( ( main_window && firstTime && !Amarok::config()->readBoolEntry( "HiddenOnExit", false ) ) || ( main_window && !AmarokConfig::showTrayIcon() ) )
{
- mainWindow()->show();
+ main_window->show();
//takes longer but feels shorter. Crazy eh? :)
kapp->eventLoop()->processEvents( QEventLoop::ExcludeUserInput );
diff --git a/src/collectionbrowser.cpp b/src/collectionbrowser.cpp
index 0135432..6226079 100644
--- a/src/collectionbrowser.cpp
+++ b/src/collectionbrowser.cpp
@@ -4278,6 +4278,9 @@ DividerItem::text(int column) const
int
DividerItem::compare( QListViewItem* i, int col, bool ascending ) const
{
+ if (!i) {
+ return QString::localeAwareCompare( text(col).lower(), QString("") );
+ }
if (dynamic_cast<CollectionItem*>(i)) {
return -1 * i->compare(const_cast<DividerItem*>(this), col, ascending);
}
diff --git a/src/magnatunebrowser/magnatunebrowser.cpp b/src/magnatunebrowser/magnatunebrowser.cpp
index f29ead1..0c56926 100644
--- a/src/magnatunebrowser/magnatunebrowser.cpp
+++ b/src/magnatunebrowser/magnatunebrowser.cpp
@@ -174,6 +174,10 @@ void MagnatuneBrowser::selectionChanged(
{
// a track is selected, show the corrosponding album info!
MagnatuneListViewTrackItem *trackItem = dynamic_cast<MagnatuneListViewTrackItem*>( item );
+ if (!trackItem) {
+ debug() << "dynamic_cast to trackItem failed!" << endl;
+ return;
+ }
int albumId = trackItem->getAlbumId();
MagnatuneAlbum album = MagnatuneDatabaseHandler::instance() ->getAlbumById( albumId );
m_artistInfobox->displayAlbumInfo( &album );
@@ -271,6 +275,10 @@ void MagnatuneBrowser::purchaseAlbumCont
}
MagnatuneListViewTrackItem *selectedTrack = dynamic_cast<MagnatuneListViewTrackItem *>( m_listView->selectedItem() );
+ if (!selectedTrack) {
+ debug() << "dynamic_cast to selected track failed!" << endl;
+ return;
+ }
MagnatuneAlbum *album = new MagnatuneAlbum( MagnatuneDatabaseHandler::instance() ->getAlbumById( selectedTrack->getAlbumId() ) );
diff --git a/src/magnatunebrowser/magnatunelistview.cpp b/src/magnatunebrowser/magnatunelistview.cpp
index b5b8b8a..1e9be84 100644
--- a/src/magnatunebrowser/magnatunelistview.cpp
+++ b/src/magnatunebrowser/magnatunelistview.cpp
@@ -53,6 +53,10 @@ KURLDrag * MagnatuneListView::dragObject
MagnatuneTrackList::iterator it;
KListViewItem * pSelectedItem = dynamic_cast<KListViewItem *>( selectedItem() );
+ if (!pSelectedItem) {
+ debug() << "dynamic_cast to pSelectedItem failed!" << endl;
+ return 0;
+ }
switch ( pSelectedItem->depth() )
{
diff --git a/src/mediabrowser.cpp b/src/mediabrowser.cpp
index 5f537f1..a5bb1db 100644
--- a/src/mediabrowser.cpp
+++ b/src/mediabrowser.cpp
@@ -1054,7 +1054,7 @@ MediaItem::compare( QListViewItem *i, in
MediaItem *item = dynamic_cast<MediaItem *>(i);
if(item && col==0 && item->m_order != m_order)
return m_order-item->m_order;
- else if( item->type() == MediaItem::ARTIST )
+ else if( item && item->type() == MediaItem::ARTIST )
{
QString key1 = key( col, ascending );
if( key1.startsWith( "the ", false ) )
diff --git a/src/mediadevice/daap/daapclient.cpp b/src/mediadevice/daap/daapclient.cpp
index f15f4cf..d68df7e 100644
--- a/src/mediadevice/daap/daapclient.cpp
+++ b/src/mediadevice/daap/daapclient.cpp
@@ -515,6 +515,10 @@ DaapClient::passwordPrompt()
};
Daap::Reader* callback = dynamic_cast<Daap::Reader*>( const_cast<QObject*>( sender() ) );
+ if (!callback) {
+ debug() << "No callback!" << endl;
+ return;
+ }
ServerItem* root = callback->rootMediaItem();
PasswordDialog dialog( 0 );
diff --git a/src/playlistbrowser.cpp b/src/playlistbrowser.cpp
index 5726892..447fadc 100644
--- a/src/playlistbrowser.cpp
+++ b/src/playlistbrowser.cpp
@@ -2073,10 +2073,12 @@ void PlaylistBrowser::removeSelectedItem
if( parent && parent->isSelected() ) //parent will remove children
continue;
- while( parent && parent->parent() && static_cast<PlaylistBrowserEntry*>(parent)->isKept() )
- parent = parent->parent();
+ if (parent) {
+ while( parent->parent() && static_cast<PlaylistBrowserEntry*>(parent)->isKept() )
+ parent = parent->parent();
+ }
- if( !static_cast<PlaylistBrowserEntry*>(parent)->isKept() )
+ if( parent && !static_cast<PlaylistBrowserEntry*>(parent)->isKept() )
continue;
switch( (*it)->rtti() )
diff --git a/src/playlistwindow.cpp b/src/playlistwindow.cpp
index 31bfd3c..ae00eb0 100644
--- a/src/playlistwindow.cpp
+++ b/src/playlistwindow.cpp
@@ -718,7 +718,9 @@ bool PlaylistWindow::eventFilter( QObjec
if( pl->currentItem() && ( e->key() == Key_Up && pl->currentItem()->itemAbove() == 0 ) )
{
QListViewItem *lastitem = *It( pl, It::Visible );
- while( lastitem && lastitem->itemBelow() )
+ if ( !lastitem )
+ return false;
+ while( lastitem->itemBelow() )
lastitem = lastitem->itemBelow();
pl->currentItem()->setSelected( false );
pl->setCurrentItem( lastitem );
More information about the Amarok
mailing list