Bug 92610

CVS commit by staikos: 

Delete groups properly and fix child removal
BUG: 92610


  M +6 -4      kstviewobject.cpp   1.96


--- kdeextragear-2/kst/kst/kstviewobject.cpp  #1.95:1.96
@@ -277,11 +277,9 @@ bool KstViewObject::removeChild(KstViewO
   if (rc == 0 && recursive) {
     for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) {
-      if ((*i)->removeChild(obj, true)) {
-        obj->_parent = 0L;
-        return true;
-      }
+      (*i)->removeChild(obj, true);
     }
   }
 
+  obj->_parent = 0L;
   return rc > 0;
 }
@@ -723,4 +721,8 @@ void KstViewObject::deleteObject() {
     _parent->removeChild(this);
   }
+  for (KstViewObjectList::Iterator it = _children.begin(); it != _children.end(); ++it) {
+    (*it)->_parent = 0L;
+  }
+  _children.clear();
   QTimer::singleShot(0, KstApp::inst(), SLOT(updateDialogs()));
 }

This change seems to have introduced a crash.

TO REPRODUCE:
Create a new window with 2 or more plots.
Delete one of plots.

RESULT:
Crash

On Wednesday 03 November 2004 15:39, Andrew Walker wrote:
> ------- This change seems to have introduced a crash.
>
> TO REPRODUCE:
> Create a new window with 2 or more plots.
> Delete one of plots.
>
> RESULT:
> Crash

  Do you have a backtrace?

CVS commit by staikos: 

attempt #2 to fix all the deletion/ref issues with view objects

BUG: 92610


  M +4 -4      kstplotgroup.cpp   1.23
  M +11 -7     kstviewobject.cpp   1.97
  M +2 -1      kstviewobject.h   1.80


--- kdeextragear-2/kst/kst/kstplotgroup.cpp  #1.22:1.23
@@ -65,10 +65,8 @@ void KstPlotGroup::removeFocus(QPainter&
 
 bool KstPlotGroup::removeChild(KstViewObjectPtr obj, bool recursive) {
-  KstViewObjectList::Iterator it;
-
   if (KstViewObject::removeChild(obj, recursive)) {
     if (_children.count() > 1) {
       QRect gg = _children.first()->geometry();
-      for (it = _children.begin(); it != _children.end(); ++it) {
+      for (KstViewObjectList::Iterator it = _children.begin(); it != _children.end(); ++it) {
         gg |= (*it)->geometry();
       }
@@ -80,6 +78,8 @@ bool KstPlotGroup::removeChild(KstViewOb
       }
     } else {
+      if (_parent) { // can be false if we are being deleted already
       flatten();
     }
+    }
 
     return true;

--- kdeextragear-2/kst/kst/kstviewobject.cpp  #1.96:1.97
@@ -37,4 +37,5 @@
 
 KstViewObject::KstViewObject(const QString& type) : KstObject(), _geom(0, 0, 1, 1), _type(type) {
+  _parent = 0L;
   _standardActions = 0;
   _layoutActions = 0;
@@ -53,4 +54,5 @@ KstViewObject::KstViewObject(QDomElement
   _foregroundColor = KstSettings::globalSettings()->foregroundColor;
   _backgroundColor = KstSettings::globalSettings()->backgroundColor;
+  _parent = 0L;
   load(e);
 }
@@ -274,13 +276,13 @@ void KstViewObject::prependChild(KstView
 
 bool KstViewObject::removeChild(KstViewObjectPtr obj, bool recursive) {
-  uint rc = _children.remove(obj);
-  if (rc == 0 && recursive) {
+  bool rc = _children.remove(obj) > 0;
+  if (recursive) {
     for (KstViewObjectList::Iterator i = _children.begin(); i != _children.end(); ++i) {
-      (*i)->removeChild(obj, true);
+      rc = rc && (*i)->removeChild(obj, true);
     }
   }
 
   obj->_parent = 0L;
-  return rc > 0;
+  return rc;
 }
 
@@ -718,11 +720,13 @@ void KstViewObject::deleteObject() {
   // FIXME: eliminate _parent!!
   KstApp::inst()->document()->setModified();
+  KstViewObjectPtr vop(this);
   if (_parent) {
     _parent->removeChild(this);
+    _parent = 0L;
   }
-  for (KstViewObjectList::Iterator it = _children.begin(); it != _children.end(); ++it) {
-    (*it)->_parent = 0L;
+  while (!_children.isEmpty()) {
+    removeChild(_children.first());
   }
-  _children.clear();
+  vop = 0L; // basically "delete this;"
   QTimer::singleShot(0, KstApp::inst(), SLOT(updateDialogs()));
 }

--- kdeextragear-2/kst/kst/kstviewobject.h  #1.79:1.80
@@ -219,5 +219,6 @@ class KstViewObject : public KstObject {
     bool _maximized : 1;
     int _columns : 6; // "64 columns ought to be enough for anyone"
-    KstViewObjectPtr _parent; // FIXME: this is bad and should be removed ASAP
+    KstViewObject *_parent; // danger!!
+    //KstViewObjectPtr _parent; // FIXME: this is bad and should be removed ASAP
                               //        It was introduced as a temporary hack
                               //        but is no longer needed as events now