* Jim Wilson -- Monday 09 May 2005 14:27:
> Sounds good.  Are there any performance implications?  Off hand it doesn't 
> sound like it. 

No, doesn't sound like it. The only change is in removeChild() (which isn't
used often), and when reading XML files (which is only done at startup). No
runtime changes. 



> Maybe a test doing a bunch of switches back and forth between  
> different types just to check for leakage would be good.

I'm using the internal clear_value(), which (I hope) does already care for
that. It does only reset the value to a default and then set the type to
NONE, so that the setters "dare" to assign a new type. No new allocations.


> Am I right though that since the SetXXXValue alone won't change, this
> clear will not be done frequently?

Exactly.



> Or do we have code that will (maybe incorrectly) do this every frame?

No. Removing nodes is probably one of the least often used functions.  :-)


BTW: I changed the patch: I was overly cautious: clear_value() does already
care for ties and for setting NONE, so we just need to make that public as
clearValue(), and use that. Makes the patch a bit more verbose, though.  :-/

m.


Index: props.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/SimGear/simgear/props/props.cxx,v
retrieving revision 1.7
diff -u -p -r1.7 props.cxx
--- props.cxx	23 Dec 2004 13:32:01 -0000	1.7
+++ props.cxx	9 May 2005 12:52:12 -0000
@@ -471,7 +471,7 @@ SGPropertyNode::set_string (const char *
 }
 
 void
-SGPropertyNode::clear_value ()
+SGPropertyNode::clearValue ()
 {
   switch (_type) {
   case NONE:
@@ -762,7 +762,7 @@ SGPropertyNode::~SGPropertyNode ()
   delete [] _display_name;
   delete [] _path;
   delete _path_cache;
-  clear_value();
+  clearValue();
   delete _listeners;
 }
 
@@ -775,7 +775,7 @@ SGPropertyNode::alias (SGPropertyNode * 
 {
   if (target == 0 || _type == ALIAS || _tied)
     return false;
-  clear_value();
+  clearValue();
   _value.alias = target;
   _type = ALIAS;
   return true;
@@ -928,6 +928,7 @@ SGPropertyNode::removeChild (const char 
       _removedChildren.push_back(node);
     }
     node->setAttribute(REMOVED, true);
+    node->clearValue();
     ret = node;
     fireChildRemoved(node);
   }
@@ -1168,7 +1169,7 @@ SGPropertyNode::setBoolValue (bool value
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _tied = false;
     _type = BOOL;
   }
@@ -1216,7 +1217,7 @@ SGPropertyNode::setIntValue (int value)
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _type = INT;
     _local_val.int_val = 0;
   }
@@ -1267,7 +1268,7 @@ SGPropertyNode::setLongValue (long value
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _type = LONG;
     _local_val.long_val = 0L;
   }
@@ -1318,7 +1319,7 @@ SGPropertyNode::setFloatValue (float val
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _type = FLOAT;
     _local_val.float_val = 0;
   }
@@ -1369,7 +1370,7 @@ SGPropertyNode::setDoubleValue (double v
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _local_val.double_val = value;
     _type = DOUBLE;
   }
@@ -1420,7 +1421,7 @@ SGPropertyNode::setStringValue (const ch
   bool result = false;
   TEST_WRITE;
   if (_type == NONE || _type == UNSPECIFIED) {
-    clear_value();
+    clearValue();
     _type = STRING;
   }
 
@@ -1464,7 +1465,7 @@ SGPropertyNode::setUnspecifiedValue (con
   bool result = false;
   TEST_WRITE;
   if (_type == NONE) {
-    clear_value();
+    clearValue();
     _type = UNSPECIFIED;
   }
 
@@ -1513,7 +1514,7 @@ SGPropertyNode::tie (const SGRawValue<bo
   if (useDefault)
     old_val = getBoolValue();
 
-  clear_value();
+  clearValue();
   _type = BOOL;
   _tied = true;
   _value.bool_val = rawValue.clone();
@@ -1535,7 +1536,7 @@ SGPropertyNode::tie (const SGRawValue<in
   if (useDefault)
     old_val = getIntValue();
 
-  clear_value();
+  clearValue();
   _type = INT;
   _tied = true;
   _value.int_val = rawValue.clone();
@@ -1557,7 +1558,7 @@ SGPropertyNode::tie (const SGRawValue<lo
   if (useDefault)
     old_val = getLongValue();
 
-  clear_value();
+  clearValue();
   _type = LONG;
   _tied = true;
   _value.long_val = rawValue.clone();
@@ -1579,7 +1580,7 @@ SGPropertyNode::tie (const SGRawValue<fl
   if (useDefault)
     old_val = getFloatValue();
 
-  clear_value();
+  clearValue();
   _type = FLOAT;
   _tied = true;
   _value.float_val = rawValue.clone();
@@ -1601,7 +1602,7 @@ SGPropertyNode::tie (const SGRawValue<do
   if (useDefault)
     old_val = getDoubleValue();
 
-  clear_value();
+  clearValue();
   _type = DOUBLE;
   _tied = true;
   _value.double_val = rawValue.clone();
@@ -1624,7 +1625,7 @@ SGPropertyNode::tie (const SGRawValue<co
   if (useDefault)
     old_val = getStringValue();
 
-  clear_value();
+  clearValue();
   _type = STRING;
   _tied = true;
   _value.string_val = rawValue.clone();
@@ -1644,35 +1645,35 @@ SGPropertyNode::untie ()
   switch (_type) {
   case BOOL: {
     bool val = getBoolValue();
-    clear_value();
+    clearValue();
     _type = BOOL;
     _local_val.bool_val = val;
     break;
   }
   case INT: {
     int val = getIntValue();
-    clear_value();
+    clearValue();
     _type = INT;
     _local_val.int_val = val;
     break;
   }
   case LONG: {
     long val = getLongValue();
-    clear_value();
+    clearValue();
     _type = LONG;
     _local_val.long_val = val;
     break;
   }
   case FLOAT: {
     float val = getFloatValue();
-    clear_value();
+    clearValue();
     _type = FLOAT;
     _local_val.float_val = val;
     break;
   }
   case DOUBLE: {
     double val = getDoubleValue();
-    clear_value();
+    clearValue();
     _type = DOUBLE;
     _local_val.double_val = val;
     break;
@@ -1680,7 +1681,7 @@ SGPropertyNode::untie ()
   case STRING:
   case UNSPECIFIED: {
     string val = getStringValue();
-    clear_value();
+    clearValue();
     _type = STRING;
     _local_val.string_val = copy_string(val.c_str());
     break;
Index: props.hxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/SimGear/simgear/props/props.hxx,v
retrieving revision 1.6
diff -u -p -r1.6 props.hxx
--- props.hxx	23 Dec 2004 13:32:01 -0000	1.6
+++ props.hxx	9 May 2005 12:52:13 -0000
@@ -1172,6 +1172,12 @@ public:
   void fireChildRemoved (SGPropertyNode * child);
 
 
+  /**
+   * Clear any existing value and set the type to NONE.
+   */
+  void clearValue ();
+
+
 protected:
 
   void fireValueChanged (SGPropertyNode * node);
@@ -1204,12 +1210,6 @@ private:
 
 
   /**
-   * Clear any existing value and set the type to NONE.
-   */
-  void clear_value ();
-
-
-  /**
    * Get the value as a string.
    */
   const char * make_string () const;
Index: props_io.cxx
===================================================================
RCS file: /var/cvs/SimGear-0.3/SimGear/simgear/props/props_io.cxx,v
retrieving revision 1.7
diff -u -p -r1.7 props_io.cxx
--- props_io.cxx	23 Dec 2004 13:32:01 -0000	1.7
+++ props_io.cxx	9 May 2005 12:52:13 -0000
@@ -173,6 +173,7 @@ PropsVisitor::startElement (const char *
 
 				// Got the index, so grab the node.
     SGPropertyNode * node = st.node->getChild(name, index, true);
+    node->clearValue();
 
 				// Get the access-mode attributes,
 				// but don't set yet (in case they
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@flightgear.org
http://mail.flightgear.org/mailman/listinfo/flightgear-devel
2f585eeea02e2c79d7b1d8c4963bae2d

Reply via email to