Bug 146748

Confirmed with 1.5.71 and current branches/KDE/3.5/kdesdk/umbrello

Created attachment 20850 [details]
valgrind --tool=memcheck --num-callers=25

SVN commit 675229 by okellogg:

loadChildrenFromXMI, (parent != item->parent()): Prohibit reparenting m_datatypeFolder.
The file loads now but it is very possible that there are more weak spots.
In that case please provide further attachments and reopen.
BUG:146748


 M  +1 -0      ChangeLog  
 M  +13 -9     umbrello/umllistview.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/ChangeLog #675228:675229
@@ -2,6 +2,7 @@
 
 * Bugs/wishes from http://bugs.kde.org:
 * Wrong pascal code generation (146676)
+* Crash when linking to undefined xmi.id (146748)
 
 Version 1.5.71
 
--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umllistview.cpp #675228:675229
@@ -2389,8 +2389,7 @@
             // Pull a new ID now.
             nID = UniqueID::get();
         } else {
-            kError() << "UMLListView::loadChildrenFromXMI: item of type "
-                << type << " has no ID, skipping." << endl;
+            kError() << pfx << "item of type " << type << " has no ID, skipping." << endl;
             domElement = node.toElement();
             continue;
         }
@@ -2451,14 +2450,19 @@
                 // listview item might be located in a user created folder.
                 // Thanks to Achim Spangler for spotting the problem.
                 UMLListViewItem *itmParent = dynamic_cast<UMLListViewItem*>(item->parent());
-                kDebug() << pfx << parent->getText() << " (" << parent << ") != "
+                kDebug() << pfx << item->getText() << " parent "
+                    << parent->getText() << " (" << parent << ") != "
                     << itmParent->getText() << " (" << itmParent << ")" << endl;
-                UMLListViewItem *newItem = moveObject(nID, lvType, parent);
-                item = newItem;
-                if (item) {
-                    kDebug() << pfx << "Attempted reparenting of " << item->getText()
-                        << "(current parent: " << (itmParent ? itmParent->getText() : "NULL")
-                        << ", new parent: " << parent->getText() << ")" << endl;
+                if (item == m_datatypeFolder && itmParent == m_lv[Uml::mt_Logical]) {
+                    kDebug() << pfx << "Reparenting the Datatypes folder is prohibited" << endl;
+                } else {
+                    UMLListViewItem *newItem = moveObject(nID, lvType, parent);
+                    item = newItem;
+                    if (item) {
+                        kDebug() << pfx << "Attempted reparenting of " << item->getText()
+                            << "(current parent: " << (itmParent ? itmParent->getText() : "NULL")
+                            << ", new parent: " << parent->getText() << ")" << endl;
+                    }
                 }
             }
             break;

SVN commit 675723 by okellogg:

moveObject(): Don't physically delete listview items while reparenting during
 load from XMI.
CCBUG:146748


 M  +32 -8     umllistview.cpp  


--- branches/KDE/3.5/kdesdk/umbrello/umbrello/umllistview.cpp #675722:675723
@@ -1284,7 +1284,10 @@
         if (newParentType == Uml::lvt_UseCase_Folder ||
                 newParentType == Uml::lvt_UseCase_View) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1294,7 +1297,10 @@
         if (newParentType == Uml::lvt_Component_Folder ||
                 newParentType == Uml::lvt_Component_View) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1303,7 +1309,10 @@
             newParentType == Uml::lvt_Component_View ||
             newParentType == Uml::lvt_Subsystem) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1313,7 +1322,10 @@
             newParentType == Uml::lvt_Component ||
             newParentType == Uml::lvt_Subsystem) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1323,7 +1335,10 @@
         if (newParentType == Uml::lvt_Deployment_Folder ||
                 newParentType == Uml::lvt_Deployment_View) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1333,7 +1348,10 @@
         if (newParentType == Uml::lvt_EntityRelationship_Folder ||
                 newParentType == Uml::lvt_EntityRelationship_Model) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1346,7 +1364,10 @@
         if (newParentType == Uml::lvt_Logical_Folder ||
                 newParentType == Uml::lvt_Logical_View) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             addAtContainer(newItem, newParent);
         }
         break;
@@ -1362,7 +1383,10 @@
                 newParentType == Uml::lvt_Interface ||
                 newParentType == Uml::lvt_Package) {
             newItem = move->deepCopy(newParent);
-            delete move;
+            if (m_doc->loading())         // deletion is not safe while loading
+                move->setVisible(false);  // (the <listview> XMI may be corrupted)
+            else
+                delete move;
             UMLCanvasObject *o = static_cast<UMLCanvasObject*>(newItem->getUMLObject());
             if (o == NULL) {
                 kDebug() << "moveObject: newItem's UMLObject is NULL" << endl;