Change request #689

GXmlNode append method should clone node.

Added by Knödlseder Jürgen over 11 years ago. Updated over 10 years ago.

Status:ClosedStart date:01/11/2013
Priority:NormalDue date:
Assigned To:Knödlseder Jürgen% Done:

100%

Category:-
Target version:00-08-00
Duration:

Description

The GXmlNode method actually appends a pointer to a node, hence if the node gets deleted, the result becomes invalid.

The following Python code leads to a segmentation fault:

            spectrum = GXmlElement()
            spectrum.attribute("type", "ExpCutoff")
            spectrum.append(prefactor)
            spectrum.append(index)
            spectrum.append(cutoff)
            spectrum.append(scale)

but the following code works
            spectrum = GXmlElement()
            spectrum.attribute("type", "ExpCutoff")
            spectrum.append(prefactor.clone())
            spectrum.append(index.clone())
            spectrum.append(cutoff.clone())
            spectrum.append(scale.clone())

The append method should always clone the node. Instead of passing a pointer, a reference should be passed.


Recurrence

No recurrence.


Related issues

Related to GammaLib - Feature #692: Perform an extensive interface review of all classes Closed 01/12/2013

History

#1 Updated by Knödlseder Jürgen over 11 years ago

  • Target version set to 00-08-00

#2 Updated by Knödlseder Jürgen about 11 years ago

I’m not really sure about this anymore, at least it requires careful consideration.

For example, the GModelSky::write relies on appending a pointer. The code

    // If no source with corresponding name was found then append one
    if (src == NULL) {
        src = new GXmlElement("source");
        src->attribute("name") = name();
        if (spectral() != NULL) src->append(new GXmlElement("spectrum"));
        if (spatial()  != NULL) src->append(new GXmlElement("spatialModel"));
        xml.append(src); // Appends pointer
    }

    // Set model attributes
    src->attribute("name", name());
    src->attribute("type", type());
    std::string instruments = this->instruments();
    if (instruments.length() > 0) {
        src->attribute("instrument", instruments);
    }

assumes that src still handles the object that has been appended to xml.

THIS CHANGE REQUEST NEEDS CAREFUL EVALUATION, AND A GENERAL STRATEGY SHOULD BE DEFINED FOR CONTAINERS OF POINTERS. One consideration is that memory leaks should be avoided. This is not possible if a container simply handles pointers in a dumb way.

#3 Updated by Knödlseder Jürgen over 10 years ago

  • Status changed from New to Closed
  • Assigned To set to Knödlseder Jürgen

GXmlNode now clones.

#4 Updated by Knödlseder Jürgen over 10 years ago

  • % Done changed from 0 to 100

Also available in: Atom PDF