Opened 9 years ago

Last modified 6 years ago

#6143 closed enhancement

Propose CPLXMLTreeCloser — at Initial Version

Reported by: Kurt Schwehr Owned by: warmerdam
Priority: normal Milestone:
Component: default Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

I propose this as an initial start at making cleanup simpler. This mimics the style of std::unique_ptr.

// Manage a tree of XML nodes so that all nodes are freed when the instance goes
// out of scope.  Only the top level node should be in a CPLXMLTreeCloser.
class CPLXMLTreeCloser {
   public:
    explicit CPLXMLTreeCloser(CPLXMLNode* data) { the_data_ = data; }

    ~CPLXMLTreeCloser() {
        if (the_data_) CPLDestroyXMLNode(the_data_);
    }

    // Modifying the contents pointed to by the return is allowed.
    CPLXMLNode* get() const { return the_data_; }

    CPLXMLNode* operator->() const { return get(); }

   private:
    CPLXMLNode* the_data_;
};

// Example of use in a test gunit test case.
TEST(CplMiniXmlTest, ParseOne) {
    CPLXMLTreeCloser root(CPLParseXMLString("<a/>"));
    ASSERT_NE(nullptr, root.get());
    EXPECT_EQ(CXT_Element, root->eType);
    EXPECT_STREQ("a", root->pszValue);
    EXPECT_EQ(nullptr, root->psNext);
    EXPECT_EQ(nullptr, root->psChild);
}

An example of how this can be used to simplify cleanup in functions/methods with multiple return paths needing cleanup. (picking ParseKMLGeometry from ogrgftlayer.cpp at random)

static OGRGeometry* ParseKMLGeometry(const char* pszKML)
{
    CPLXMLNode* psXML = CPLParseXMLString(pszKML);
    if (psXML == NULL)  // Add .get() to fetch the pointer.
        return NULL;

    if (psXML->eType != CXT_Element)  // Member access works the same.
    {
        CPLDestroyXMLNode(psXML);  // This will go away.
        return NULL;
    }

    OGRGeometry* poGeom = ParseKMLGeometry(psXML);  // Add .get() to fetch pointer.

    CPLDestroyXMLNode(psXML);  // This will go away.
    return poGeom;
}

becomes (is the o prefix correct?)

static OGRGeometry* ParseKMLGeometry(const char* pszKML)
{
    CPLXMLTreeCloser oXML = CPLParseXMLString(pszKML);
    if (oXML.get() == NULL)
        return NULL;

    if (oXML->eType != CXT_Element)
    {
        return NULL;
    }

    OGRGeometry* poGeom = ParseKMLGeometry(oXML.get());

    return poGeom;
}

Or even simpler:

static OGRGeometry* ParseKMLGeometry(const char* pszKML)
{
    CPLXMLTreeCloser psXML = CPLParseXMLString(pszKML);
    if (psXML.get() == NULL || psXML->eType != CXT_Element)
    {
        return NULL;
    }

    return ParseKMLGeometry(psXML.get());  // ParseKMLGeometry(CPLXMLNode* psXML).
}

Change History (0)

Note: See TracTickets for help on using tickets.