kmix: fixed memory leak associated with alsa. Various code improvements.

Signed-off-by: Michele Calgaro <michele.calgaro@yahoo.it>

4 files changed, 112 insertions, 111 deletions

diff --git a/kmix/KMixApp.cpp b/kmix/KMixApp.cpp
index 40bf6cfb..d511605c 100644
--- a/kmix/KMixApp.cpp
+++ b/kmix/KMixApp.cpp
@@ -35,7 +35,10 @@ KMixApp::KMixApp()
 
 KMixApp::~KMixApp()
 {
-   delete m_kmix;
+  if (m_kmix)
+  {
+    delete m_kmix;
+  }
 }
 
 
diff --git a/kmix/kmix.cpp b/kmix/kmix.cpp
index 1ac09d85..3ebf6729 100644
--- a/kmix/kmix.cpp
+++ b/kmix/kmix.cpp
@@ -101,9 +101,7 @@ KMixWindow::~KMixWindow()
    MixerToolBox::deinitMixer();
 }
 
-
-void
-KMixWindow::initActions()
+void KMixWindow::initActions()
 {
 	// file menu
 	KStdAction::quit( TQT_TQOBJECT(this), TQT_SLOT(quit()), actionCollection());
@@ -131,18 +129,14 @@ KMixWindow::initActions()
 	createGUI( "kmixui.rc" );
 }
 
-
-void
-KMixWindow::initPrefDlg()
+void KMixWindow::initPrefDlg()
 {
 	m_prefDlg = new KMixPrefDlg( this );
 	connect( m_prefDlg, TQT_SIGNAL(signalApplied(KMixPrefDlg *)),
 			this, TQT_SLOT(applyPrefs(KMixPrefDlg *)) );
 }
 
-
-void
-KMixWindow::initWidgets()
+void KMixWindow::initWidgets()
 {
 	// Main widget and layout
 	setCentralWidget( new TQWidget(  this, "qt_central_widget" ) );
@@ -180,9 +174,7 @@ KMixWindow::initWidgets()
 	widgetsLayout->activate();
 }
 
-
-void
-KMixWindow::updateDocking()
+void KMixWindow::updateDocking()
 {
 	// delete old dock widget
 	if (m_dockWidget)
@@ -199,15 +191,13 @@ KMixWindow::updateDocking()
 	}
 }
 
-void
-KMixWindow::saveSettings()
+void KMixWindow::saveSettings()
 {
     saveConfig();
     saveVolumes();
 }
 
-void
-KMixWindow::saveConfig()
+void KMixWindow::saveConfig()
 {
     TDEConfig *config = kapp->config();
     config->setGroup(0);
@@ -266,8 +256,7 @@ KMixWindow::saveConfig()
    config->setGroup(0);
 }
 
-void
-KMixWindow::loadConfig()
+void KMixWindow::loadConfig()
 {
     TDEConfig *config = kapp->config();
     config->setGroup(0);
@@ -327,18 +316,13 @@ KMixWindow::loadConfig()
 	}
 }
 
-
-void
-KMixWindow::initMixerWidgets()
+void KMixWindow::initMixerWidgets()
 {
-   m_mixerWidgets.clear();
+   m_mixerWidgets.clear(); // This auto delete all KMixerWidget objects
+   m_cMixer->clear();
 
-	int id=0;
-	Mixer *mixer;
-
-	// Attention!! If Mixer::mixers() is empty, we behave stupid. We don't show nothing and there
-        //             is not even a context menu.
-	for ( mixer=Mixer::mixers().first(),id=0; mixer!=0; mixer=Mixer::mixers().next(),id++ )
+	int id = 0;
+	for (Mixer *mixer = Mixer::mixers().first(); mixer; mixer = Mixer::mixers().next(), id++)
 	{
 		//kdDebug(67100) << "Mixer number: " << id << " Name: " << mixer->mixerName() << endl ;
 		ViewBase::ViewFlags vflags = ViewBase::HasMenuBar;
@@ -370,25 +354,23 @@ KMixWindow::initMixerWidgets()
 		TQString grp;
 		grp.sprintf( "%i", mw->id() );
 		mw->loadConfig( kapp->config(), grp );
-
 		mw->setTicks( m_showTicks );
 		mw->setLabels( m_showLabels );
 		mw->setValueStyle ( m_valueStyle );
-		// !! I am still not sure whether this works 100% reliably - chris
 		mw->show();
 	}
 
-	if (id == 1)
+	if (m_cMixer->count() > 1)
+	{
+		mixerNameLayout->show();
+	}
+	else
 	{
-		// don't show the Current Mixer bit unless we have multiple mixers
 		mixerNameLayout->hide();
 	}
 }
 
-
-
-bool
-KMixWindow::queryClose ( )
+bool KMixWindow::queryClose ( )
 {
     if ( m_showDockWidget && !kapp->sessionSaving() )
     {
@@ -398,16 +380,12 @@ KMixWindow::queryClose ( )
     return true;
 }
 
-
-void
-KMixWindow::quit()
+void KMixWindow::quit()
 {
 	kapp->quit();
 }
 
-
-void
-KMixWindow::showSettings()
+void KMixWindow::showSettings()
 {
    if (!m_prefDlg->isVisible())
    {
@@ -428,16 +406,12 @@ KMixWindow::showSettings()
    }
 }
 
-
-void
-KMixWindow::showHelp()
+void KMixWindow::showHelp()
 {
 	actionCollection()->action( "help_contents" )->activate();
 }
 
-
-void
-KMixWindow::showAbout()
+void KMixWindow::showAbout()
 {
    actionCollection()->action( "help_about_app" )->activate();
 }
@@ -465,8 +439,7 @@ KMixWindow::loadVolumes()
  * Stores the volumes of all mixers  Can be restored via loadVolumes() or
  * the kmixctrl application.
  */
-void
-KMixWindow::saveVolumes()
+void KMixWindow::saveVolumes()
 {
     TDEConfig *cfg = new TDEConfig( "kmixctrlrc", false );
     for (Mixer *mixer=Mixer::mixers().first(); mixer!=0; mixer=Mixer::mixers().next()) {
@@ -478,9 +451,7 @@ KMixWindow::saveVolumes()
     delete cfg;
 }
 
-
-void
-KMixWindow::applyPrefs( KMixPrefDlg *prefDlg )
+void KMixWindow::applyPrefs( KMixPrefDlg *prefDlg )
 {
    m_showDockWidget = prefDlg->m_dockingChk->isChecked();
    m_volumeWidget = prefDlg->m_volumeChk->isChecked();
@@ -539,9 +510,7 @@ KMixWindow::applyPrefs( KMixPrefDlg *prefDlg )
    saveConfig();
 }
 
-
-void
-KMixWindow::toggleMenuBar()
+void KMixWindow::toggleMenuBar()
 {
 	m_showMenubar = !m_showMenubar;
 	if( m_showMenubar )
@@ -554,16 +523,14 @@ KMixWindow::toggleMenuBar()
 	}
 }
 
-void
-KMixWindow::showEvent( TQShowEvent * )
+void KMixWindow::showEvent( TQShowEvent * )
 {
     if ( m_visibilityUpdateAllowed )
 	m_isVisible = isVisible();
     // !! could possibly start polling now (idea: use someting like ref() and unref() on Mixer instance
 }
 
-void
-KMixWindow::hideEvent( TQHideEvent * )
+void KMixWindow::hideEvent( TQHideEvent * )
 {
     if ( m_visibilityUpdateAllowed )
     {
@@ -574,9 +541,8 @@ KMixWindow::hideEvent( TQHideEvent * )
     //    But the dock icon is currently no View, so that must be done first.
 }
 
-
-void
-KMixWindow::stopVisibilityUpdates() {
+void KMixWindow::stopVisibilityUpdates()
+{
     m_visibilityUpdateAllowed = false;
 }
 
@@ -585,14 +551,12 @@ KMixWindow::slotHWInfo() {
 	KMessageBox::information( 0, m_hwInfoString, i18n("Mixer Hardware Information") );
 }
 
-void
-KMixWindow::showSelectedMixer( int mixer )
+void KMixWindow::showSelectedMixer( int mixer )
 {
 	m_wsMixers->raiseWidget( mixer );
 }
 
-void
-KMixWindow::configureGlobalShortcuts()
+void KMixWindow::configureGlobalShortcuts()
 {
 	KKeyDialog::configure( m_globalAccel, 0, false ) ;
         m_globalAccel->writeSettings();
diff --git a/kmix/mixer.cpp b/kmix/mixer.cpp
index 62b6430a..778d6af9 100644
--- a/kmix/mixer.cpp
+++ b/kmix/mixer.cpp
@@ -80,7 +80,9 @@ bool Mixer::isValid(int driver, int device)
     Mixer_Backend *mb = mf(device);
     if (mb)
     {
-      return mb->isValid();
+      bool ret = mb->isValid();
+      delete mb;
+      return ret;
     }
   }
   return false;
@@ -92,8 +94,8 @@ Mixer::Mixer( int driver, int device ) : DCOPObject( "Mixer" )
 
    _mixerBackend = 0;
    getMixerFunc *f = g_mixerFactories[driver].getMixer;
-   if( f!=0 ) {
-     _mixerBackend = f( device );
+   if (f) {
+     _mixerBackend = f(device );
    }
 
   readSetFromHWforceUpdate();  // enforce an initial update on first readSetFromHW()
@@ -120,6 +122,10 @@ Mixer::~Mixer() {
    // Close the mixer. This might also free memory, depending on the called backend method
    close();
    delete _pollingTimer;
+   if (_mixerBackend)
+   {
+     delete _mixerBackend;
+   }
 }
 
 void Mixer::volumeSave( TDEConfig *config )
diff --git a/kmix/mixer_alsa9.cpp b/kmix/mixer_alsa9.cpp
index 54af7161..2a37051d 100644
--- a/kmix/mixer_alsa9.cpp
+++ b/kmix/mixer_alsa9.cpp
@@ -105,14 +105,6 @@ Mixer_ALSA::open()
     bool masterChosen = false;
     int err;
 
-    snd_ctl_t *ctl_handle;
-    snd_ctl_card_info_t *hw_info;
-    snd_ctl_card_info_alloca(&hw_info);
-
-    snd_mixer_elem_t *elem;
-    snd_mixer_selem_id_t *sid;
-    snd_mixer_selem_id_alloca( &sid );
-
     // Card information
     if ((unsigned)m_devnum > 31) m_devnum = -1;
     devName = m_devnum == -1 ? "default" : TQString("hw:%1").arg(m_devnum);
@@ -122,6 +114,7 @@ Mixer_ALSA::open()
     if (virginOpen)
 	probeMessage += "Trying ALSA Device '" + devName + "': ";
 
+    snd_ctl_t *ctl_handle;
     if ( ( err = snd_ctl_open ( &ctl_handle, devName.latin1(), 0 ) ) < 0 )
     {
 	kdDebug(67100) << probeMessage << "not found: snd_ctl_open err=" << snd_strerror(err) << endl;
@@ -129,23 +122,29 @@ Mixer_ALSA::open()
 	return Mixer::ERR_OPEN;
     }
 
-    if ( ( err = snd_ctl_card_info ( ctl_handle, hw_info ) ) < 0 )
+    snd_ctl_card_info_t *hw_info;
+    snd_ctl_card_info_malloc(&hw_info);
+    if ((err = snd_ctl_card_info(ctl_handle, hw_info)) < 0)
     {
 	kdDebug(67100) << probeMessage << "not found: snd_ctl_card_info err=" << snd_strerror(err) << endl;
 	//_stateMessage = errorText( Mixer::ERR_READ );
-	snd_ctl_close( ctl_handle );
+	snd_ctl_card_info_free(hw_info);
+	snd_ctl_close(ctl_handle);
 	return Mixer::ERR_READ;
     }
 
     // Device and mixer names
-    const char* mixer_card_name =  snd_ctl_card_info_get_name( hw_info );
+    const char* mixer_card_name = snd_ctl_card_info_get_name( hw_info );
     //mixer_device_name = snd_ctl_card_info_get_mixername( hw_info );
     // Copy the name of kmix mixer from card name (mixername is rumoured to be not that good)
     m_mixerName = mixer_card_name;
     if (m_devnum == -1) m_devnum = snd_card_get_index(snd_ctl_card_info_get_id(hw_info));
     if (m_devnum < 0) m_devnum = -1;
 
-    snd_ctl_close( ctl_handle );
+    // NOTE do not free hw_info here, it cause funny detections of mixers in subsequent updates.
+    //      Instead free the resources later, before leaving this function.
+    // snd_ctl_card_info_free(hw_info);
+    snd_ctl_close(ctl_handle);
 
     /* open mixer device */
 
@@ -155,6 +154,7 @@ Mixer_ALSA::open()
 	kdDebug(67100) << probeMessage << "not found: snd_mixer_open err=" << snd_strerror(err) << endl;
 	//errormsg( Mixer::ERR_NODEV );
 	_handle = 0;
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_NODEV; // if we cannot open the mixer, we have no devices
     }
     //kdDebug(67100) << "OUT Mixer_ALSA snd_mixer_open()" << endl;
@@ -163,6 +163,8 @@ Mixer_ALSA::open()
     {
 	kdDebug(67100) << probeMessage << "not found: snd_mixer_attach err=" << snd_strerror(err) << endl;
 	//errormsg( Mixer::ERR_PERM );
+	close();
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_OPEN;
     }
 
@@ -170,6 +172,8 @@ Mixer_ALSA::open()
     {
 	kdDebug(67100) << probeMessage << "not found: snd_mixer_selem_register err=" << snd_strerror(err) << endl;
 	//errormsg( Mixer::ERR_READ );
+	close();
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_READ;
     }
 
@@ -178,27 +182,26 @@ Mixer_ALSA::open()
 	kdDebug(67100) << probeMessage << "not found: snd_mixer_load err=" << snd_strerror(err) << endl;
 	//errormsg( Mixer::ERR_READ );
 	close();
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_READ;
     }
 
     kdDebug(67100) << probeMessage << "found" << endl;
 
     unsigned int mixerIdx = 0;
-    for ( elem = snd_mixer_first_elem( _handle ); elem; elem = snd_mixer_elem_next( elem ), mixerIdx++ )
+    snd_mixer_elem_t *elem;
+    for ( elem = snd_mixer_first_elem(_handle); elem; elem = snd_mixer_elem_next(elem), mixerIdx++ )
     {
 	// If element is not active, just skip
-	if ( ! snd_mixer_selem_is_active ( elem ) ) {
+	if ( !snd_mixer_selem_is_active(elem))
+	{
 	    // ...but we still want to insert a null value into our mixer element
 	    // list so that the list indexes match up.
-	    mixer_elem_list.append( 0 );
-	    mixer_sid_list.append( 0 );
+	    mixer_elem_list.append(0);
+	    mixer_sid_list.append(0);
 	    continue;
 	}
 
-
-	sid = (snd_mixer_selem_id_t*)malloc(snd_mixer_selem_id_sizeof());  // I believe *we* must malloc it for ourself
-	snd_mixer_selem_get_id( elem, sid );
-
 	bool canPlay = false;
 	bool canCapture = false;
 	bool hasPlaySwitch = false;
@@ -209,8 +212,13 @@ Mixer_ALSA::open()
 
 	snd_mixer_selem_get_playback_volume_range( elem, &minVolumePlay, &maxVolumePlay );
 	snd_mixer_selem_get_capture_volume_range( elem, &minVolumeRec , &maxVolumeRec  );
+
 	// New mix device
-	MixDevice::ChannelType ct = (MixDevice::ChannelType)identify( sid );
+	snd_mixer_selem_id_t *sid;
+	snd_mixer_selem_id_malloc(&sid);
+	snd_mixer_selem_get_id(elem, sid);
+
+	MixDevice::ChannelType ct = (MixDevice::ChannelType)identify(sid);
 /*
         if (!masterChosen && ct==MixDevice::VOLUME) {
            // Determine a nicer MasterVolume
@@ -243,7 +251,7 @@ Mixer_ALSA::open()
 		    if ( ret == 0 ) {
 		      TQString* enumName = new TQString(buffer);
 		      //enumName->append(buffer);
-		      enumList.append( enumName);
+		      enumList.append(enumName);
 		    } // enumName could be read succesfully
 		  } // for all enum items of this device
 		} // no error in reading enum list
@@ -252,7 +260,6 @@ Mixer_ALSA::open()
 		}
 		// --- get Enum names END ---
 	    } // is an enum
-
 	    else {
 		Volume::ChannelMask chn = Volume::MNONE;
 		Volume::ChannelMask chnTmp;
@@ -346,19 +353,23 @@ Mixer_ALSA::open()
 	    //kdDebug(67100) << "ALSA create MDW, vol= " << *vol << endl;
 	    delete volPlay;
 	    delete volCapture;
-	} // virginOpen
+	}
 	else
 	{
-	    MixDevice* md;
+	    snd_mixer_selem_id_free(sid);
 	    bool found = false;
-	    for ( md = m_mixDevices.first(); md != 0; md = m_mixDevices.next() ) {
-		if ( md->num() == mixerIdx ) {
+	    for (MixDevice *md = m_mixDevices.first(); md; md = m_mixDevices.next())
+	    {
+		if (md->num() == mixerIdx)
+		{
 		    found = true;
 		    writeVolumeToHW( mixerIdx, md->getVolume() );
 		}
 	    }
 	    if( !found )
 	    {
+		close();
+		snd_ctl_card_info_free(hw_info);
 		return Mixer::ERR_INCOMPATIBLESET;
 	    }
 	} // !virginOpen
@@ -372,6 +383,8 @@ Mixer_ALSA::open()
     ***************************************************************************************/
     if ( !validDevice )
     {
+	close();
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_NODEV;
     }
 
@@ -385,25 +398,33 @@ Mixer_ALSA::open()
     /* setup for select on stdin and the mixer fd */
     if ((m_count = snd_mixer_poll_descriptors_count(_handle)) < 0) {
 	kdDebug(67100) << "Mixer_ALSA::poll() , snd_mixer_poll_descriptors_count() err=" <<  m_count << "\n";
+	close();
+	snd_ctl_card_info_free(hw_info);
 	return Mixer::ERR_OPEN;
     }
 
+    // Finally release the hw_info resources here!
+    snd_ctl_card_info_free(hw_info);
+
     //kdDebug(67100) << "Mixer_ALSA::prepareUpdate() 2\n";
 
     m_fds = (struct pollfd*)calloc(m_count, sizeof(struct pollfd));
     if (m_fds == NULL) {
 	kdDebug(67100) << "Mixer_ALSA::poll() , calloc() = null" << "\n";
-        return Mixer::ERR_OPEN;
+	close();
+	return Mixer::ERR_OPEN;
     }
 
     m_fds->events = POLLIN;
     if ((err = snd_mixer_poll_descriptors(_handle, m_fds, m_count)) < 0) {
 	kdDebug(67100) << "Mixer_ALSA::poll() , snd_mixer_poll_descriptors_count() err=" <<  err << "\n";
-        return Mixer::ERR_OPEN;
+	close();
+	return Mixer::ERR_OPEN;
     }
     if (err != m_count) {
 	kdDebug(67100) << "Mixer_ALSA::poll() , snd_mixer_poll_descriptors_count() err=" << err << " m_count=" <<  m_count << "\n";
-        return Mixer::ERR_OPEN;
+	close();
+	return Mixer::ERR_OPEN;
     }
 
     return 0;
@@ -442,32 +463,39 @@ Mixer_ALSA::close()
 {
   int ret=0;
   m_isOpen = false;
-  if ( _handle != 0 )
+
+  removeSignalling();
+
+  if (_handle)
   {
     //kdDebug(67100) << "IN  Mixer_ALSA::close()" << endl;
-    snd_mixer_free ( _handle );
-    if ( ( ret = snd_mixer_detach ( _handle, devName.latin1() ) ) < 0 )
+    snd_mixer_free(_handle);
+    if ((ret = snd_mixer_detach(_handle, devName.latin1())) < 0)
     {
         kdDebug(67100) << "snd_mixer_detach err=" << snd_strerror(ret) << endl;
     }
     int ret2 = 0;
-    if ( ( ret2 = snd_mixer_close ( _handle ) ) < 0 )
+    if ((ret2 = snd_mixer_close(_handle)) < 0)
     {
         kdDebug(67100) << "snd_mixer_close err=" << snd_strerror(ret2) << endl;
-	if ( ret == 0 ) ret = ret2; // no error before => use current error code
+        if (ret == 0) ret = ret2; // no error before => use current error code
     }
-
     _handle = 0;
     //kdDebug(67100) << "OUT Mixer_ALSA::close()" << endl;
-
   }
 
+  while (!mixer_sid_list.isEmpty())
+  {
+    snd_mixer_selem_id_t *currSid = mixer_sid_list.first();
+    if (currSid)
+    {
+	snd_mixer_selem_id_free(currSid);
+    }
+    mixer_sid_list.pop_front();
+  }
   mixer_elem_list.clear();
-  mixer_sid_list.clear();
   m_mixDevices.clear();
 
-  removeSignalling();
-
   return ret;
 }