| Summary: | memory leak of src/mediumpluginmanager.cpp | ||
|---|---|---|---|
| Product: | [Applications] amarok | Reporter: | Erik Hovland <erik> |
| Component: | general | Assignee: | Amarok Developers <amarok-bugs-dist> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | ||
| Priority: | NOR | ||
| Version: | 1.4-SVN | ||
| Target Milestone: | --- | ||
| Platform: | Mandriva RPMs | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | Fixes the medium object memory leak | ||
|
Description
Erik Hovland
2007-02-02 01:05:47 UTC
Created attachment 19505 [details]
Fixes the medium object memory leak
This patch adds a pointer var to assign the provided medium to. Since the
medium that is gotten is probably the one that the code wants to use again in
the next if, the object pointed to with that pointer var is then used to
determine if the medium name is a QString null.
After that block is finished the medium object is deleted before the scope of
the original conditional is finished.
SVN commit 629248 by mitchell:
Fix mem leaks in ManualDeviceAdder and do some cleanup. This code compiles cleanly but I haven't been able to test yet...since
1.4.5 is already tagged though, there's plenty of time... :-)
For 1.4.
CCBUG: 141036
M +19 -9 mediumpluginmanager.cpp
M +2 -3 mediumpluginmanager.h
--- branches/stable/extragear/multimedia/amarok/src/mediumpluginmanager.cpp #629247:629248
@@ -229,7 +229,7 @@
{
DEBUG_BLOCK
ManualDeviceAdder* mda = new ManualDeviceAdder( this );
- if( mda->exec() == QDialog::Accepted && mda->successful() && mda->getMedium() != 0 )
+ if( mda->exec() == QDialog::Accepted && mda->successful() )
{
if( !Amarok::config( "MediaBrowser" )->readEntry( mda->getMedium()->id() ).isNull() )
{
@@ -240,10 +240,9 @@
}
else
{
- Medium *newdev = mda->getMedium();
+ Medium *newdev = new Medium( mda->getMedium() );
Amarok::config( "MediaBrowser" )->writeEntry( newdev->id(), mda->getPlugin() );
MediaDeviceManager::instance()->addManualDevice( newdev );
- m_newDevMap[newdev->id()] = newdev;
detectDevices();
}
}
@@ -258,6 +257,7 @@
{
m_mpm = mpm;
m_successful = false;
+ m_newMed = 0;
kapp->setTopWidget( this );
setCaption( kapp->makeStdCaption( i18n( "Add New Device") ) );
@@ -296,6 +296,7 @@
ManualDeviceAdder::~ManualDeviceAdder()
{
+ delete m_newMed;
delete m_mdaName;
delete m_mdaMountPoint;
}
@@ -309,7 +310,7 @@
void
ManualDeviceAdder::slotOk()
{
- if( getMedium() && !getMedium()->name().isEmpty() &&
+ if( getMedium( true ) && !getMedium()->name().isEmpty() &&
MediaDeviceManager::instance()->getDevice( getMedium()->name() ) == NULL )
{
m_successful = true;
@@ -348,8 +349,17 @@
}
Medium*
-ManualDeviceAdder::getMedium()
+ManualDeviceAdder::getMedium( bool recreate )
{
+ if( !recreate )
+ return m_newMed;
+
+ if( m_newMed && recreate )
+ {
+ delete m_newMed;
+ m_newMed = 0;
+ }
+
if( m_mdaMountPoint->isEnabled() == false &&
m_mdaName->text().isNull() )
return NULL;
@@ -360,10 +370,10 @@
( m_mdaMountPoint->text().isNull() ||
m_mdaMountPoint->isEnabled() == false ?
"(null)" : m_mdaMountPoint->text() );
- Medium* added = new Medium( id, m_mdaName->text() );
- added->setAutodetected( false );
- added->setMountPoint( m_mdaMountPoint->text() );
- return added;
+ m_newMed = new Medium( id, m_mdaName->text() );
+ m_newMed->setAutodetected( false );
+ m_newMed->setMountPoint( m_mdaMountPoint->text() );
+ return m_newMed;
}
MediaDeviceConfig::MediaDeviceConfig( Medium *medium, MediumPluginManager *mgr, const bool nographics, QWidget *parent, const char *name )
--- branches/stable/extragear/multimedia/amarok/src/mediumpluginmanager.h #629247:629248
@@ -35,7 +35,6 @@
class MediumPluginManager;
typedef QMap<QString, Medium*> DeletedMap;
-typedef QMap<QString, Medium*> NewDeviceMap;
/**
@author Jeff Mitchell <kde-dev@emailgoeshere.com>
@@ -105,7 +104,6 @@
private:
bool detectDevices( bool redetect=false, bool nographics=false );
DeletedMap m_deletedMap;
- NewDeviceMap m_newDevMap;
DeviceList m_deviceList;
QWidget *m_widget;
bool m_hasChanged;
@@ -138,7 +136,7 @@
ManualDeviceAdder( MediumPluginManager* mdm );
~ManualDeviceAdder();
bool successful() const { return m_successful; }
- Medium* getMedium();
+ Medium* getMedium( bool recreate = false );
QString getPlugin() const { return m_selectedPlugin; }
private slots:
@@ -151,6 +149,7 @@
bool m_successful;
QString m_comboOldText;
QString m_selectedPlugin;
+ Medium *m_newMed;
KComboBox* m_mdaCombo;
HintLineEdit* m_mdaName;
SVN commit 629249 by mitchell:
Fix mem leaks in ManualDeviceAdder and do some cleanup.
For 2.0
BUG: 141036
M +19 -9 mediumpluginmanager.cpp
M +2 -3 mediumpluginmanager.h
--- trunk/extragear/multimedia/amarok/src/mediumpluginmanager.cpp #629248:629249
@@ -229,7 +229,7 @@
{
DEBUG_BLOCK
ManualDeviceAdder* mda = new ManualDeviceAdder( this );
- if( mda->exec() == QDialog::Accepted && mda->successful() && mda->getMedium() != 0 )
+ if( mda->exec() == QDialog::Accepted && mda->successful() )
{
if( !Amarok::config( "MediaBrowser" )->readEntry( mda->getMedium()->id() ).isNull() )
{
@@ -240,10 +240,9 @@
}
else
{
- Medium *newdev = mda->getMedium();
+ Medium *newdev = new Medium( mda->getMedium() );
Amarok::config( "MediaBrowser" )->writeEntry( newdev->id(), mda->getPlugin() );
MediaDeviceManager::instance()->addManualDevice( newdev );
- m_newDevMap[newdev->id()] = newdev;
detectDevices();
}
}
@@ -258,6 +257,7 @@
{
m_mpm = mpm;
m_successful = false;
+ m_newMed = 0;
kapp->setTopWidget( this );
setCaption( KInstance::makeStandardCaption( i18n( "Add New Device") ) );
@@ -297,6 +297,7 @@
ManualDeviceAdder::~ManualDeviceAdder()
{
+ delete m_newMed;
delete m_mdaName;
delete m_mdaMountPoint;
}
@@ -310,7 +311,7 @@
void
ManualDeviceAdder::slotOk()
{
- if( getMedium() && !getMedium()->name().isEmpty() &&
+ if( getMedium( true ) && !getMedium()->name().isEmpty() &&
MediaDeviceManager::instance()->getDevice( getMedium()->name() ) == NULL )
{
m_successful = true;
@@ -349,8 +350,17 @@
}
Medium*
-ManualDeviceAdder::getMedium()
+ManualDeviceAdder::getMedium( bool recreate )
{
+ if( !recreate )
+ return m_newMed;
+
+ if( m_newMed && recreate )
+ {
+ delete m_newMed;
+ m_newMed = 0;
+ }
+
if( m_mdaMountPoint->isEnabled() == false &&
m_mdaName->text().isNull() )
return NULL;
@@ -361,10 +371,10 @@
( m_mdaMountPoint->text().isNull() ||
m_mdaMountPoint->isEnabled() == false ?
"(null)" : m_mdaMountPoint->text() );
- Medium* added = new Medium( id, m_mdaName->text() );
- added->setAutodetected( false );
- added->setMountPoint( m_mdaMountPoint->text() );
- return added;
+ m_newMed = new Medium( id, m_mdaName->text() );
+ m_newMed->setAutodetected( false );
+ m_newMed->setMountPoint( m_mdaMountPoint->text() );
+ return m_newMed;
}
MediaDeviceConfig::MediaDeviceConfig( Medium *medium, MediumPluginManager *mgr, const bool nographics, QWidget *parent, const char *name )
--- trunk/extragear/multimedia/amarok/src/mediumpluginmanager.h #629248:629249
@@ -38,7 +38,6 @@
class MediumPluginManager;
typedef QMap<QString, Medium*> DeletedMap;
-typedef QMap<QString, Medium*> NewDeviceMap;
/**
@author Jeff Mitchell <kde-dev@emailgoeshere.com>
@@ -108,7 +107,6 @@
private:
bool detectDevices( bool redetect=false, bool nographics=false );
DeletedMap m_deletedMap;
- NewDeviceMap m_newDevMap;
DeviceList m_deviceList;
QWidget *m_widget;
bool m_hasChanged;
@@ -141,7 +139,7 @@
ManualDeviceAdder( MediumPluginManager* mdm );
~ManualDeviceAdder();
bool successful() const { return m_successful; }
- Medium* getMedium();
+ Medium* getMedium( bool recreate = false );
QString getPlugin() const { return m_selectedPlugin; }
private slots:
@@ -154,6 +152,7 @@
bool m_successful;
QString m_comboOldText;
QString m_selectedPlugin;
+ Medium *m_newMed;
KComboBox* m_mdaCombo;
HintLineEdit* m_mdaName;
|