Bug 92610 - Deleting a plot group does not decrement usage count of curves.
Summary: Deleting a plot group does not decrement usage count of curves.
Status: RESOLVED FIXED
Alias: None
Product: kst
Classification: Applications
Component: view objects (show other bugs)
Version: 1.x
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: kst
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-11-02 21:28 UTC by Netterfield
Modified: 2004-11-04 17:37 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Netterfield 2004-11-02 21:28:40 UTC
Version:           1.0.0_pre1 (using KDE 3.3.0, Gentoo)
Compiler:          gcc version 3.3.4 20040623 (Gentoo Linux 3.3.4-r1, ssp-3.3.2-2, pie-8.7.6)
OS:                Linux (i686) release 2.6.8-gentoo-r3

How to reproduce:
-create several plots with curves.
-group a couple of them.
-rmb->delete in the group.
-look at the usage count - it didn't go down.

Correct behavior: the usage count should go down.
Comment 1 George Staikos 2004-11-02 23:32:44 UTC
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()));
 }


Comment 2 Andrew Walker 2004-11-03 21:39:57 UTC
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
Comment 3 George Staikos 2004-11-03 23:01:56 UTC
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?

Comment 4 George Staikos 2004-11-04 17:37:42 UTC
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