[umbrello] [Bug 368282] New: Make type casting more robust

[Ralf Habacker via KDE Bugzilla]

https://bugs.kde.org/show_bug.cgi?id=368282

            Bug ID: 368282
           Summary: Make type casting more robust
           Product: umbrello
           Version: 2.20.1 (KDE Applications 16.08.1)
          Platform: Other
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: NOR
         Component: general
          Assignee: umbrello-devel at kde.org
          Reporter: ralf.habacker at freenet.de

Umbrello UML model is designed by using several classes named UML.... Mostly of
them are based on class UMLObject. Storing uml model objects are saved in lists
of type UMLObject* for example in UMLPackage::m_objects and casted back to the
original type by using a static_cast. In several places a type check is done
before casting by using UMLObject::baseType()  to make sure not to perform
invalid casts. While working on the code in the past it turns out that this
does not work in any cases. A prominent example is in bool
UMLClassifier::resolveRef() 
... 
        if (obj->resolveRef()) {
            UMLClassifierListItem *cli =
static_cast<UMLClassifierListItem*>(obj);
            switch (cli->baseType()) {

where 'obj' is casted to UMLClassifierListItem.  Mostly objects are not of type
UMLClassifierListItem, which results into an invalid type case. Using the
related code to use a dynamic cast

        if (obj->resolveRef()) {
            UMLClassifierListItem *cli =
dynamic_cast<UMLClassifierListItem*>(obj);
            switch (cli->baseType()) {

return zero, but let umbrello crash on accessing cli->baseType() on loading a
bigger xmi file. To complete the fix the return value from dynamic_cast needs
to be checked as shown below.

        if (obj->resolveRef()) {
            UMLClassifierListItem *cli =
dynamic_cast<UMLClassifierListItem*>(obj);
            if (cli)
                return false;
            switch (cli->baseType()) {

These issues need to be fixed in the code. Because several of them are reported
by the coverity scan there is already a list of related locations.

Another issue with directly using casts is that it could be applied to any
object, although it may be not applicable as shown in the following example. In
 UMLOperation::toString() there is the following code
... 
  UMLClassifier *ownParent = static_cast<UMLClassifier*>(parent());
... 
which looks like a call to get the parent UML model object.  In fact parent()
if of type QObject (the uml model parent is returned with umlPackage()) and the
static_cast returns an invalid cast. Also dynamic_cast would fail (it will
return 0, which is not expected) 

To avoid such issues and for simplier writing of such fragements class
UMLObject should get type wrappers containing casts to related classes as shown
in the following example.

  UMLClassifier *ownParent = parent()->asUMLClassifier();

The benefit of having this style is that the compiler detects that parent() is
not of type UMLObject and fails to compile with error: 'class QObject' has no
member named 'asUMLClassifier', so we are informed at compile time of this
issue.

Reproducible: Always

-- 
You are receiving this mail because:
You are the assignee for the bug.