Patch from Frederic Bouvier:

I have modified the SGPropertyNode interface to support node removal
and address the issues you mention yesterday.

I added the SGPropertyNode_ptr class that is a smart pointer that
increment and decrement a counter in SGPropertyNode. The _children
vector is changed to a vector<SGPropertyNode_ptr> in order for the
node to hold a reference to its children. I also added a
_removedChildren vector of the same type. When removeChild is called
with the keep parameter set to true, the reference is removed from
_children and inserted in _removedChildren, and the attribute REMOVED
is set. When getChild is called with create set to true, the _children
vector is searched, then the _removedChildren and then a new node is
created. If a resurrected node is returned, the REMOVED bit is cleared.
The static const int LAST_USED_ATTRIBUTE is also added.

The footprint of this patch is light. SGPropertyNode is one int
longer in size, SGPropertyNode_ptr is one pointer large without
virtual functions. Some functions can be inlined if your current
policy change.

The FlightGear patch is to take account the change in the getChildren
function that now returns a vector<SGPropertyNode_ptr>. If the
removeChild functionnality is to be added in FlightGear, all those
SGPropertyNode * floating around should be changed to
SGPropertyNode_ptr.

diff --git a/simgear/misc/props.cxx b/simgear/misc/props.cxx
index 880d73511665e8f82433dbec51a88afd1325d15e..fd1fb6c3fb78e92cec917080632c34d3338f1de5 100644 (file)
--- a/simgear/misc/props.cxx
+++ b/simgear/misc/props.cxx
@@ -40,7 +40,7 @@ SG_USING_STD(sort);
 class CompareIndices
 {
 public:
-  int operator() (const SGPropertyNode * n1, const SGPropertyNode *n2) const {
+  int operator() (const SGPropertyNode_ptr n1, const SGPropertyNode_ptr n2) const {
     return (n1->getIndex() < n2->getIndex());
   }
 };
@@ -236,7 +236,7 @@ compare_strings (const char * s1, const char * s2)
  * Locate a child node by name and index.
  */
 static int
-find_child (const char * name, int index, vector<SGPropertyNode *> nodes)
+find_child (const char * name, int index, vector<SGPropertyNode_ptr> nodes)
 {
   int nNodes = nodes.size();
   for (int i = 0; i < nNodes; i++) {
@@ -551,12 +551,36 @@ SGPropertyNode::trace_read () const
 #endif
 }
 
+/**
+ * Increment reference counter
+ */
+void
+SGPropertyNode::incrementRef()
+{
+  ++_count;
+}
+
+/**
+ * Decrement reference counter
+ */
+int
+SGPropertyNode::decrementRef()
+{
+  return --_count;
+}
+
 
 \f
 ////////////////////////////////////////////////////////////////////////
 // Public methods from SGPropertyNode.
 ////////////////////////////////////////////////////////////////////////
 
+/**
+ * Last used attribute
+ * Update as needed when enum Attribute is changed
+ */
+const int SGPropertyNode::LAST_USED_ATTRIBUTE = TRACE_WRITE;
+
 /**
  * Default constructor: always creates a root node.
  */
@@ -567,7 +591,8 @@ SGPropertyNode::SGPropertyNode ()
     _path_cache(0),
     _type(NONE),
     _tied(false),
-    _attr(READ|WRITE)
+    _attr(READ|WRITE),
+    _count(0)
 {
   _local_val.string_val = 0;
 }
@@ -582,7 +607,8 @@ SGPropertyNode::SGPropertyNode (const SGPropertyNode &node)
     _path_cache(0),
     _type(node._type),
     _tied(node._tied),
-    _attr(node._attr)
+    _attr(node._attr),
+    _count(0)
 {
   _name = copy_string(node._name);
   _local_val.string_val = 0;
@@ -663,7 +689,8 @@ SGPropertyNode::SGPropertyNode (const char * name,
     _path_cache(0),
     _type(NONE),
     _tied(false),
-    _attr(READ|WRITE)
+    _attr(READ|WRITE),
+    _count(0)
 {
   _name = copy_string(name);
   _local_val.string_val = 0;
@@ -676,9 +703,6 @@ SGPropertyNode::SGPropertyNode (const char * name,
 SGPropertyNode::~SGPropertyNode ()
 {
   delete [] _name;
-  for (int i = 0; i < (int)_children.size(); i++) {
-    delete _children[i];
-  }
   delete _path_cache;
   clear_value();
 }
@@ -776,7 +800,18 @@ SGPropertyNode::getChild (const char * name, int index, bool create)
   if (pos >= 0) {
     return _children[pos];
   } else if (create) {
-    _children.push_back(new SGPropertyNode(name, index, this));
+    SGPropertyNode_ptr node;
+    pos = find_child(name, index, _removedChildren);
+    if (pos >= 0) {
+      std::vector<SGPropertyNode_ptr>::iterator it = _removedChildren.begin();
+      it += pos;
+      node = _removedChildren[pos];
+      _removedChildren.erase(it);
+      node->setAttribute(REMOVED, false);
+    } else {
+      node = new SGPropertyNode(name, index, this);
+    }
+    _children.push_back(node);
     return _children[_children.size()-1];
   } else {
     return 0;
@@ -801,10 +836,10 @@ SGPropertyNode::getChild (const char * name, int index) const
 /**
  * Get all children with the same name (but different indices).
  */
-vector<SGPropertyNode *>
-SGPropertyNode::getChildren (const char * name)
+vector<SGPropertyNode_ptr>
+SGPropertyNode::getChildren (const char * name) const
 {
-  vector<SGPropertyNode *> children;
+  vector<SGPropertyNode_ptr> children;
   int max = _children.size();
 
   for (int i = 0; i < max; i++)
@@ -817,20 +852,25 @@ SGPropertyNode::getChildren (const char * name)
 
 
 /**
- * Get all children const with the same name (but different indices).
+ * Remove a child node
  */
-vector<const SGPropertyNode *>
-SGPropertyNode::getChildren (const char * name) const
+SGPropertyNode_ptr 
+SGPropertyNode::removeChild (const char * name, int index, bool keep)
 {
-  vector<const SGPropertyNode *> children;
-  int max = _children.size();
-
-  for (int i = 0; i < max; i++)
-    if (compare_strings(_children[i]->getName(), name))
-      children.push_back(_children[i]);
-
-  sort(children.begin(), children.end(), CompareIndices());
-  return children;
+  SGPropertyNode_ptr ret;
+  int pos = find_child(name, index, _children);
+  if (pos >= 0) {
+    std::vector<SGPropertyNode_ptr>::iterator it = _children.begin();
+    it += pos;
+    SGPropertyNode_ptr node = _children[pos];
+    _children.erase(it);
+    if (keep) {
+      _removedChildren.push_back(node);
+    }
+    node->setAttribute(REMOVED, true);
+    ret = node;
+  }
+  return ret;
 }
 
 
@@ -2024,4 +2064,102 @@ SGPropertyNode::hash_table::hashcode (const char * key)
   return hash;
 }
 
+
+
+/**
+ * Default constructor
+ */
+SGPropertyNode_ptr::SGPropertyNode_ptr()
+{
+  _ptr = 0;
+}
+
+/**
+ * Copy constructor
+ */
+SGPropertyNode_ptr::SGPropertyNode_ptr( const SGPropertyNode_ptr &r )
+{
+  _ptr = r._ptr;
+  if (_ptr)
+     _ptr->incrementRef();
+}
+
+/**
+ * Constructor from a pointer to a node
+ */
+SGPropertyNode_ptr::SGPropertyNode_ptr( SGPropertyNode *p )
+{
+  _ptr = p;
+  if (_ptr)
+     _ptr->incrementRef();
+}
+
+/**
+ * Destructor
+ */
+SGPropertyNode_ptr::~SGPropertyNode_ptr()
+{
+  if (_ptr && _ptr->decrementRef() == 0)
+    delete _ptr;
+}
+
+/**
+ * Assignement operator
+ */
+SGPropertyNode_ptr &
+SGPropertyNode_ptr::operator=( const SGPropertyNode_ptr &r )
+{
+  if (_ptr && _ptr->decrementRef() == 0)
+    delete _ptr;
+  _ptr = r._ptr;
+  if (_ptr)
+     _ptr->incrementRef();
+
+  return *this;
+}
+
+/**
+ * Pointer access operator
+ */
+SGPropertyNode *
+SGPropertyNode_ptr::operator->()
+{
+  return _ptr;
+}
+
+/**
+ * Pointer access operator (const)
+ */
+const SGPropertyNode *
+SGPropertyNode_ptr::operator->() const
+{
+  return _ptr;
+}
+
+/**
+ * Conversion to SGPropertyNode * operator
+ */
+SGPropertyNode_ptr::operator SGPropertyNode *()
+{
+  return _ptr;
+}
+
+/**
+ * Conversion to const SGPropertyNode * operator
+ */
+SGPropertyNode_ptr::operator const SGPropertyNode *() const
+{
+  return _ptr;
+}
+
+/**
+ * Validity test
+ */
+bool 
+SGPropertyNode_ptr::valid() const
+{
+  return _ptr != 0;
+}
+
+
 // end of props.cxx

diff --git a/simgear/misc/props.hxx b/simgear/misc/props.hxx
index 6a0032f69560c3d620a746a974b067ad747d8a05..a03b20066f217f29b96e11d1233a4678d5f5efd1 100644 (file)
--- a/simgear/misc/props.hxx
+++ b/simgear/misc/props.hxx
@@ -449,6 +449,69 @@ private:
   setter_t _setter;
 };
 
+\f
+/**
+ * The smart pointer that manage reference counting
+ */
+class SGPropertyNode;
+class SGPropertyNode_ptr
+{
+public:
+
+  /**
+   * Default constructor
+   */
+  SGPropertyNode_ptr();
+
+  /**
+   * Copy constructor
+   */
+  SGPropertyNode_ptr( const SGPropertyNode_ptr &r );
+
+  /**
+   * Constructor from a pointer to a node
+   */
+  SGPropertyNode_ptr( SGPropertyNode *p );
+
+  /**
+   * Destructor
+   */
+  ~SGPropertyNode_ptr();
+
+  /**
+   * Assignement operator
+   */
+  SGPropertyNode_ptr &operator=( const SGPropertyNode_ptr &r );
+
+  /**
+   * Pointer access operator
+   */
+  SGPropertyNode *operator->();
+
+  /**
+   * Pointer access operator (const)
+   */
+  const SGPropertyNode *operator->() const;
+
+  /**
+   * Conversion to SGPropertyNode * operator
+   */
+  operator SGPropertyNode *();
+
+  /**
+   * Conversion to const SGPropertyNode * operator
+   */
+  operator const SGPropertyNode *() const;
+
+  /**
+   * Validity test
+   */
+  bool valid() const;
+
+private:
+
+  SGPropertyNode *_ptr;
+};
 
 \f
 /**
@@ -491,11 +554,19 @@ public:
     READ = 1,
     WRITE = 2,
     ARCHIVE = 4,
-    TRACE_READ = 8,
-    TRACE_WRITE = 16
+    REMOVED = 8,
+    TRACE_READ = 16,
+    TRACE_WRITE = 32
   };
 
 
+  /**
+   * Last used attribute
+   * Update as needed when enum Attribute is changed
+   */
+  static const int LAST_USED_ATTRIBUTE;
+
+
   /**
    * Default constructor.
    */
@@ -588,13 +659,14 @@ public:
   /**
    * Get a vector of all children with the specified name.
    */
-  vector<SGPropertyNode *> getChildren (const char * name);
+  vector<SGPropertyNode_ptr> getChildren (const char * name) const;
 
 
   /**
-   * Get a vector all all children (const) with the specified name.
+   * Remove a child node
    */
-  vector<const SGPropertyNode *> getChildren (const char * name) const;
+  SGPropertyNode_ptr removeChild (const char * name, int index = 0,
+                                  bool keep = true);
 
 
   //
@@ -1077,6 +1149,20 @@ private:
    */
   void trace_write () const;
 
+
+  /**
+   * Increment reference counter
+   */
+  void incrementRef();
+
+  /**
+   * Decrement reference counter
+   */
+  int decrementRef();
+
+  friend class SGPropertyNode_ptr;
+
+
   mutable char _buffer[MAX_STRING_LEN+1];
 
   class hash_table;
@@ -1084,11 +1170,13 @@ private:
   char * _name;
   int _index;
   SGPropertyNode * _parent;
-  vector<SGPropertyNode *> _children;
+  vector<SGPropertyNode_ptr> _children;
+  vector<SGPropertyNode_ptr> _removedChildren;
   hash_table * _path_cache;
   Type _type;
   bool _tied;
   int _attr;
+  int _count;
 
                                // The right kind of pointer...
   union {