[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    KConfigDialog crash and fix for it
From:       "Joris Guisson" <joris.guisson () gmail ! com>
Date:       2007-10-20 11:30:25
Message-ID: 6bc528f40710200430nc20d114ia687c4b1da21d2db () mail ! gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Hi,

I have noticed that there is a crash when you use KConfigDialog. When you
add a page, then remove it again, and close the dialog with cancel (if your
press OK, it doesn't crash).

After some digging around in the code, I noticed there is this QMap
managerForPage in KConfigDialog. Which gets iterated over when you press
cancel. This map contains pointers to KConfigDialogManager objects (which
are created per page), and those should be removed when a page is removed
from the dialog, but this doesn't happen.

KConfigDialogManager goes around checking a bunch of widgets which are part
of a page and any properly coded program will delete those when it removes a
page from a KConfigDialog. So in order to fix this crash you need clean up
these KConfigDialogManager objects when the corresponding page is removed.

The attached patch does this.


Joris,

Btw, if you want to reproduce the crash, compile and install ktorrent
(trunk/extagear/network/ktorrent), load the webinterface plugin in the
settings dialog, then unload it again, and close the dialog with cancel.

[Attachment #5 (text/html)]

Hi,<br><br>I have noticed that there is a crash when you use KConfigDialog. When you \
add a page, then remove it again, and close the dialog with cancel (if your press OK, \
it doesn&#39;t crash).<br><br>After some digging around in the code, I noticed there \
is this QMap managerForPage in KConfigDialog. Which gets iterated over when you press \
cancel. This map contains pointers to KConfigDialogManager objects (which are created \
per page), and those should be removed when a page is removed from the dialog, but \
this doesn&#39;t happen.  <br><br>KConfigDialogManager goes around checking a bunch \
of widgets which are part of a page and any properly coded program will delete those \
when it removes a page from a KConfigDialog. So in order to fix this crash you need \
clean up these KConfigDialogManager objects when the corresponding page is removed. \
<br><br>The attached patch does this.<br><br><br>Joris,<br><br>Btw, if you want to \
reproduce the crash, compile and install ktorrent (trunk/extagear/network/ktorrent), \
load the webinterface plugin in the settings dialog, then unload it again, and close \
the dialog with cancel. <br><br><br>


["kconfigdialog.patch" (text/x-patch)]

Index: kdeui/paged/kpagedialog.h
===================================================================
--- kdeui/paged/kpagedialog.h	(revision 725528)
+++ kdeui/paged/kpagedialog.h	(working copy)
@@ -171,7 +171,7 @@
     /**
      * Removes the page associated with the given @see KPageWidgetItem.
      */
-    void removePage( KPageWidgetItem *item );
+    virtual void removePage( KPageWidgetItem *item );
 
     /**
      * Sets the page which is associated with the given @see KPageWidgetItem to
Index: kdeui/dialogs/kconfigdialog.cpp
===================================================================
--- kdeui/dialogs/kconfigdialog.cpp	(revision 725528)
+++ kdeui/dialogs/kconfigdialog.cpp	(working copy)
@@ -51,6 +51,7 @@
   bool shown;
   KConfigDialogManager *manager;
   QMap<QWidget *, KConfigDialogManager *> managerForPage;
+  QMap<KPageWidgetItem*, QWidget*> widgetForPageWidgetItem;
 
   /**
     * The list of existing dialogs.
@@ -152,6 +153,7 @@
   item->setIcon( KIcon( pixmapName ) );
 
   q->KPageDialog::addPage( item );
+  widgetForPageWidgetItem[item] = page;
   return item;
 }
 
@@ -166,6 +168,27 @@
     q->connect(q, SIGNAL(defaultClicked()), manager, SLOT(updateWidgetsDefault()));
 }
 
+void KConfigDialog::removePage( KPageWidgetItem *item )
+{
+	QMap<KPageWidgetItem*, QWidget*>::iterator i = d->widgetForPageWidgetItem.find(item);
+	
+	if (i != d->widgetForPageWidgetItem.end())
+	{
+		QMap<QWidget *, KConfigDialogManager *>::iterator j = d->managerForPage.find(i.value());
+		if (j != d->managerForPage.end())
+		{
+			// there is a manager for this page, so remove it
+			KConfigDialogManager* manager = j.value();
+			d->managerForPage.erase(j);
+			delete manager;
+			d->_k_updateButtons();
+		}
+		
+		d->widgetForPageWidgetItem.erase(i);
+	}
+	KPageDialog::removePage(item);
+}
+
 KConfigDialog* KConfigDialog::exists(const QString& name)
 {
   QHash<QString,KConfigDialog *>::const_iterator it = KConfigDialogPrivate::openDialogs.find( name );
Index: kdeui/dialogs/kconfigdialog.h
===================================================================
--- kdeui/dialogs/kconfigdialog.h	(revision 725528)
+++ kdeui/dialogs/kconfigdialog.h	(working copy)
@@ -161,6 +161,8 @@
    * @return True if the dialog 'name' exists and was shown.
    */
   static bool showDialog( const QString& name );
+  
+  virtual void removePage( KPageWidgetItem *item );
 
 protected Q_SLOTS:
   /**


[prev in list] [next in list] [prev in thread] [next in thread]