From 45eab946a74596bf9f87dea15e5c3cee4ae74944 Mon Sep 17 00:00:00 2001 From: Lucian Smith Date: Wed, 11 Sep 2024 17:44:52 -0700 Subject: [PATCH 1/3] Fix behavior of and memory leak from getAllElements functions. The sublist wasn't being deleted, and its contents weren't being transferred. --- src/sbml/packages/render/sbml/ListOfGlobalRenderInformation.cpp | 2 ++ src/sbml/packages/render/sbml/ListOfLocalRenderInformation.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/sbml/packages/render/sbml/ListOfGlobalRenderInformation.cpp b/src/sbml/packages/render/sbml/ListOfGlobalRenderInformation.cpp index 5e7a877ea..3c8375f0a 100644 --- a/src/sbml/packages/render/sbml/ListOfGlobalRenderInformation.cpp +++ b/src/sbml/packages/render/sbml/ListOfGlobalRenderInformation.cpp @@ -789,6 +789,8 @@ ListOfGlobalRenderInformation::getAllElements(ElementFilter* filter) { List* ret = new List(); List* sublist = ListOf::getAllElements(filter); + ret->transferFrom(sublist); + delete sublist; ADD_FILTERED_POINTER(ret, sublist, mDefaultValues, filter); diff --git a/src/sbml/packages/render/sbml/ListOfLocalRenderInformation.cpp b/src/sbml/packages/render/sbml/ListOfLocalRenderInformation.cpp index 07e18eda2..32f73f3cc 100644 --- a/src/sbml/packages/render/sbml/ListOfLocalRenderInformation.cpp +++ b/src/sbml/packages/render/sbml/ListOfLocalRenderInformation.cpp @@ -760,6 +760,8 @@ ListOfLocalRenderInformation::getAllElements(ElementFilter* filter) { List* ret = new List(); List* sublist = ListOf::getAllElements(filter); + ret->transferFrom(sublist); + delete sublist; ADD_FILTERED_POINTER(ret, sublist, mDefaultValues, filter); From 00407eb96fc6a3b00bf72fa50af0ac74c2249eee Mon Sep 17 00:00:00 2001 From: Lucian Smith Date: Wed, 11 Sep 2024 20:41:42 -0700 Subject: [PATCH 2/3] Fix RenderGroup leaks. --- src/sbml/packages/render/sbml/Style.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sbml/packages/render/sbml/Style.cpp b/src/sbml/packages/render/sbml/Style.cpp index e126f2890..84db2c734 100644 --- a/src/sbml/packages/render/sbml/Style.cpp +++ b/src/sbml/packages/render/sbml/Style.cpp @@ -601,6 +601,7 @@ Style::createGroup() delete renderns; this->setGroup(g); + delete g; connectToChild(); @@ -1280,6 +1281,7 @@ Style::createObject(XMLInputStream& stream) RenderGroup* g = new RenderGroup(renderns); g->setElementName(name); setGroup(g); + delete g; obj = &mGroup; } From b22583ea59f4c5f216d09d3064e1e6db61b27a2a Mon Sep 17 00:00:00 2001 From: Lucian Smith Date: Fri, 13 Sep 2024 19:30:05 -0700 Subject: [PATCH 3/3] At attempt to fix #398 First pass at a fix for #398, which resulted from inappropriately casting an element. Not sure I got all the use cases, but I might have? --- .../packages/render/sbml/Transformation2D.cpp | 117 ++++++++++-------- 1 file changed, 62 insertions(+), 55 deletions(-) diff --git a/src/sbml/packages/render/sbml/Transformation2D.cpp b/src/sbml/packages/render/sbml/Transformation2D.cpp index fc03e34ca..a58776d79 100644 --- a/src/sbml/packages/render/sbml/Transformation2D.cpp +++ b/src/sbml/packages/render/sbml/Transformation2D.cpp @@ -714,65 +714,72 @@ Transformation2D::addExpectedAttributes(ExpectedAttributes& attributes) */ void Transformation2D::readAttributes(const XMLAttributes& attributes, - const ExpectedAttributes& expectedAttributes) -{ - unsigned int level = getLevel(); - unsigned int version = getVersion(); - unsigned int pkgVersion = getPackageVersion(); - unsigned int numErrs; - SBMLErrorLog* log = getErrorLog(); - - if (log && getParentSBMLObject() && - static_cast(getParentSBMLObject())->size() < 2) - { - numErrs = log->getNumErrors(); - for (int n = numErrs-1; n >= 0; n--) - { - if (log->getError(n)->getErrorId() == UnknownPackageAttribute) - { - const std::string details = log->getError(n)->getMessage(); - log->remove(UnknownPackageAttribute); - log->logPackageError("render", RenderUnknown, pkgVersion, level, - version, details, getLine(), getColumn()); - } - else if (log->getError(n)->getErrorId() == UnknownCoreAttribute) - { - const std::string details = log->getError(n)->getMessage(); - log->remove(UnknownCoreAttribute); - log->logPackageError("render", RenderUnknown, pkgVersion, level, - version, details, getLine(), getColumn()); - } + const ExpectedAttributes& expectedAttributes) +{ + unsigned int level = getLevel(); + unsigned int version = getVersion(); + unsigned int pkgVersion = getPackageVersion(); + unsigned int numErrs; + SBMLErrorLog* log = getErrorLog(); + + if (log && getParentSBMLObject()) { + bool isSizeOne = false; + int type = getParentSBMLObject()->getTypeCode(); + switch (type) { + case SBML_LIST_OF: + if (static_cast(getParentSBMLObject())->size() < 2) + { + isSizeOne = true; + } + break; + case SBML_RENDER_LINEENDING: + case SBML_RENDER_STYLE_BASE: + case SBML_RENDER_LOCALSTYLE: + case SBML_RENDER_GLOBALSTYLE: + // These objects all have a single RenderGroup child, so the size can never be greater than 2. + isSizeOne = true; + break; + case SBML_RENDER_GROUP: + { + //A RenderGroup can have multiple Transformation2D children. + RenderGroup* rg = static_cast(getParentSBMLObject()); + if (rg->getNumElements() < 2) + { + isSizeOne = false; + } + break; + } + default: + assert(false); //Some situation we didn't think through; could be anything. + isSizeOne = true; + } + if (isSizeOne) { + numErrs = log->getNumErrors(); + for (int n = numErrs - 1; n >= 0; n--) + { + if (log->getError(n)->getErrorId() == UnknownPackageAttribute) + { + const std::string details = log->getError(n)->getMessage(); + log->remove(UnknownPackageAttribute); + log->logPackageError("render", RenderUnknown, pkgVersion, level, + version, details, getLine(), getColumn()); + } + else if (log->getError(n)->getErrorId() == UnknownCoreAttribute) + { + const std::string details = log->getError(n)->getMessage(); + log->remove(UnknownCoreAttribute); + log->logPackageError("render", RenderUnknown, pkgVersion, level, + version, details, getLine(), getColumn()); + } + } + } } - } - Transformation::readAttributes(attributes, expectedAttributes); - - //if (log) - //{ - // numErrs = log->getNumErrors(); - - // for (int n = numErrs-1; n >= 0; n--) - // { - // if (log->getError(n)->getErrorId() == UnknownPackageAttribute) - // { - // const std::string details = log->getError(n)->getMessage(); - // log->remove(UnknownPackageAttribute); - // log->logPackageError("render", RenderUnknown, pkgVersion, level, - // version, details); - // } - // else if (log->getError(n)->getErrorId() == UnknownCoreAttribute) - // { - // const std::string details = log->getError(n)->getMessage(); - // log->remove(UnknownCoreAttribute); - // log->logPackageError("render", - // RenderTransformation2DAllowedCoreAttributes, pkgVersion, level, - // version, details); - // } - // } - //} + Transformation::readAttributes(attributes, expectedAttributes); + std::string s; attributes.readInto("transform", s); - if(!s.empty()) + if (!s.empty()) { this->parseTransformation(s); }