todo items

4 files changed, 124 insertions, 19 deletions

diff --git a/cpp/src/Ice/BasicStream.cpp b/cpp/src/Ice/BasicStream.cpp
index 4fac52e40a5..3a99f7ab4a6 100644
--- a/cpp/src/Ice/BasicStream.cpp
+++ b/cpp/src/Ice/BasicStream.cpp
@@ -58,14 +58,14 @@ IceInternal::BasicStream::BasicStream(Instance* instance) :
 
 IceInternal::BasicStream::~BasicStream()
 {
-    while(_currentReadEncaps)
+    while(_currentReadEncaps && _currentReadEncaps != &_preAllocatedReadEncaps)
     {
 	ReadEncaps* oldEncaps = _currentReadEncaps;
 	_currentReadEncaps = _currentReadEncaps->previous;
 	delete oldEncaps;
     }
 
-    while(_currentWriteEncaps)
+    while(_currentWriteEncaps && _currentWriteEncaps != &_preAllocatedWriteEncaps)
     {
 	WriteEncaps* oldEncaps = _currentWriteEncaps;
 	_currentWriteEncaps = _currentWriteEncaps->previous;
@@ -95,8 +95,16 @@ IceInternal::BasicStream::swap(BasicStream& other)
 
     b.swap(other.b);
     std::swap(i, other.i);
+
+    //
+    // Swap is never called for BasicStreams that still have a current
+    // read or write encapsulation.
+    //
     std::swap(_currentReadEncaps, other._currentReadEncaps);
     std::swap(_currentWriteEncaps, other._currentWriteEncaps);
+    _preAllocatedReadEncaps.swap(other._preAllocatedReadEncaps);
+    _preAllocatedWriteEncaps.swap(other._preAllocatedWriteEncaps);
+
     std::swap(_seqDataStack, other._seqDataStack);
     std::swap(_objectList, other._objectList);
 }
@@ -238,7 +246,7 @@ IceInternal::BasicStream::endSeq(int sz)
 }
 
 IceInternal::BasicStream::WriteEncaps::WriteEncaps()
-    : writeIndex(0), toBeMarshaledMap(0), marshaledMap(0), typeIdMap(0), typeIdIndex(0), previous(0)
+    : writeIndex(0), toBeMarshaledMap(0), marshaledMap(0), typeIdMap(0), typeIdIndex(0)//, previous(0)
 {
 }
 
@@ -250,10 +258,46 @@ IceInternal::BasicStream::WriteEncaps::~WriteEncaps()
 }
 
 void
+IceInternal::BasicStream::WriteEncaps::reset()
+{
+    delete toBeMarshaledMap;
+    delete marshaledMap;
+    delete typeIdMap;
+
+    writeIndex = 0;
+    toBeMarshaledMap = 0;
+    marshaledMap = 0;
+    typeIdMap = 0;
+    typeIdIndex = 0;
+//    previous = 0;
+}
+
+void
+IceInternal::BasicStream::WriteEncaps::swap(WriteEncaps& other)
+{
+    std::swap(start, other.start);
+
+    std::swap(writeIndex, other.writeIndex);
+    std::swap(toBeMarshaledMap, other.toBeMarshaledMap);
+    std::swap(marshaledMap, other.marshaledMap);
+    std::swap(typeIdMap, other.typeIdMap);
+    std::swap(typeIdIndex, other.typeIdIndex);
+
+    std::swap(previous, other.previous);
+}
+
+void
 IceInternal::BasicStream::startWriteEncaps()
 {
     WriteEncaps* oldEncaps = _currentWriteEncaps;
-    _currentWriteEncaps = new WriteEncaps();
+    if(!oldEncaps) // First allocated encaps?
+    {
+	_currentWriteEncaps = &_preAllocatedWriteEncaps;
+    }
+    else
+    {
+	_currentWriteEncaps = new WriteEncaps();
+    }
     _currentWriteEncaps->previous = oldEncaps;
     _currentWriteEncaps->start = b.size();
 
@@ -286,11 +330,18 @@ IceInternal::BasicStream::endWriteEncaps()
 
     WriteEncaps* oldEncaps = _currentWriteEncaps;
     _currentWriteEncaps = _currentWriteEncaps->previous;
-    delete oldEncaps;
+    if(oldEncaps == &_preAllocatedWriteEncaps)
+    {
+	oldEncaps->reset();
+    }
+    else
+    {
+	delete oldEncaps;
+    }
 }
 
 IceInternal::BasicStream::ReadEncaps::ReadEncaps()
-    : patchMap(0), unmarshaledMap(0), typeIdMap(0), typeIdIndex(0), previous(0)
+    : patchMap(0), unmarshaledMap(0), typeIdMap(0), typeIdIndex(0)//, previous(0)
 {
 }
 
@@ -302,10 +353,48 @@ IceInternal::BasicStream::ReadEncaps::~ReadEncaps()
 }
 
 void
+IceInternal::BasicStream::ReadEncaps::reset()
+{
+    delete patchMap;
+    delete unmarshaledMap;
+    delete typeIdMap;
+
+    patchMap = 0;
+    unmarshaledMap = 0;
+    typeIdMap = 0;
+    typeIdIndex = 0;
+//    previous = 0;
+}
+
+void
+IceInternal::BasicStream::ReadEncaps::swap(ReadEncaps& other)
+{
+    std::swap(start, other.start);
+    std::swap(sz, other.sz);
+
+    std::swap(encodingMajor, other.encodingMajor);
+    std::swap(encodingMinor, other.encodingMinor);
+
+    std::swap(patchMap, other.patchMap);
+    std::swap(unmarshaledMap, other.unmarshaledMap);
+    std::swap(typeIdMap, other.typeIdMap);
+    std::swap(typeIdIndex, other.typeIdIndex);
+
+    std::swap(previous, other.previous);
+}
+
+void
 IceInternal::BasicStream::startReadEncaps()
 {
     ReadEncaps* oldEncaps = _currentReadEncaps;
-    _currentReadEncaps = new ReadEncaps();
+    if(!oldEncaps) // First allocated encaps?
+    {
+	_currentReadEncaps = &_preAllocatedReadEncaps;
+    }
+    else
+    {
+	_currentReadEncaps = new ReadEncaps();
+    }
     _currentReadEncaps->previous = oldEncaps;
     _currentReadEncaps->start = i - b.begin();
 
@@ -356,7 +445,14 @@ IceInternal::BasicStream::endReadEncaps()
 
     ReadEncaps* oldEncaps = _currentReadEncaps;
     _currentReadEncaps = _currentReadEncaps->previous;
-    delete oldEncaps;
+    if(oldEncaps == &_preAllocatedReadEncaps)
+    {
+	oldEncaps->reset();
+    }
+    else
+    {
+	delete oldEncaps;
+    }
 }
 
 void
@@ -1258,11 +1354,7 @@ IceInternal::BasicStream::read(ObjectPrx& v)
 void
 IceInternal::BasicStream::write(const ObjectPtr& v)
 {
-    if(!_currentWriteEncaps) // Lazy initialization.
-    {
-	_currentWriteEncaps = new WriteEncaps();
-	_currentWriteEncaps->start = b.size();
-    }
+    assert(_currentWriteEncaps);
 
     if(!_currentWriteEncaps->toBeMarshaledMap) // Lazy initialization.
     {
@@ -1310,10 +1402,7 @@ IceInternal::BasicStream::write(const ObjectPtr& v)
 void
 IceInternal::BasicStream::read(PatchFunc patchFunc, void* patchAddr)
 {
-    if(!_currentReadEncaps) // Lazy initialization.
-    {
-	_currentReadEncaps = new ReadEncaps();
-    }
+    assert(_currentReadEncaps);
 
     if(!_currentReadEncaps->patchMap) // Lazy initialization.
     {
diff --git a/cpp/src/Ice/Reference.cpp b/cpp/src/Ice/Reference.cpp
index 392aed1d0e6..1e626fcc0a6 100644
--- a/cpp/src/Ice/Reference.cpp
+++ b/cpp/src/Ice/Reference.cpp
@@ -498,6 +498,7 @@ IceInternal::FixedReference::changeLocator(const LocatorPrx&) const
 ReferencePtr
 IceInternal::FixedReference::changeDefault() const
 {
+    // TODO: Broken: Does not reset context, mode, and facet.
     return FixedReferencePtr(const_cast<FixedReference*>(this));
 }
 
@@ -510,6 +511,10 @@ IceInternal::FixedReference::changeCollocationOptimization(bool) const
 ReferencePtr
 IceInternal::FixedReference::changeCompress(bool) const
 {
+    // TODO: FixedReferences should probably have a _compress flag,
+    // that gets its default from the fixed connection this reference
+    // refers to. This should be changable with changeCompress(), and
+    // reset in changeDefault().
     return FixedReferencePtr(const_cast<FixedReference*>(this));
 }
 
@@ -614,6 +619,7 @@ IceInternal::RoutableReference::changeRouter(const RouterPrx& newRouter) const
 ReferencePtr
 IceInternal::RoutableReference::changeDefault() const
 {
+    // TODO: Broken: Does not reset context, mode, facet, secure, and collocationOptimization.
     RoutableReferencePtr r = RoutableReferencePtr::dynamicCast(getInstance()->referenceFactory()->copy(this));
     r->_routerInfo = getInstance()->routerManager()->get(getInstance()->referenceFactory()->getDefaultRouter());
     return r;
@@ -735,12 +741,16 @@ IceInternal::DirectReference::changeEndpoints(const vector<EndpointPtr>& newEndp
 ReferencePtr
 IceInternal::DirectReference::changeLocator(const LocatorPrx&) const
 {
+    // TODO: Broken: Does not return an IndirectReference if a non-null locator is passed.
     return DirectReferencePtr(const_cast<DirectReference*>(this));
 }
 
 ReferencePtr
 IceInternal::DirectReference::changeDefault() const
 {
+    // TODO: Broken: Does not return an IndirectReference if a default
+    // locator is set. Does not call
+    // RoutableReference::changeDefault() to change other defaults.
     return DirectReferencePtr(const_cast<DirectReference*>(this));
 }
 
@@ -922,6 +932,8 @@ IceInternal::IndirectReference::getEndpoints() const
 ReferencePtr
 IceInternal::IndirectReference::changeLocator(const LocatorPrx& newLocator) const
 {
+    // TODO: Broken: Does not return a DirectReference (with empty
+    // endpoints) if a null locator is passed.
     LocatorInfoPtr newLocatorInfo = getInstance()->locatorManager()->get(newLocator);
     if(newLocatorInfo == _locatorInfo)
     {
@@ -935,6 +947,9 @@ IceInternal::IndirectReference::changeLocator(const LocatorPrx& newLocator) cons
 ReferencePtr
 IceInternal::IndirectReference::changeDefault() const
 {
+    // TODO: Broken: Does not return an DirectReference if no default
+    // locator is set (with empty endpoints). Does not call
+    // RoutableReference::changeDefault() to change other defaults.
     IndirectReferencePtr r = IndirectReferencePtr::dynamicCast(RoutableReference::changeDefault());
     r->_locatorInfo = getInstance()->locatorManager()->get(getInstance()->referenceFactory()->getDefaultLocator());
     return r;
diff --git a/cpp/src/Ice/Reference.h b/cpp/src/Ice/Reference.h
index b4bbc14c9c3..d7a71887d55 100644
--- a/cpp/src/Ice/Reference.h
+++ b/cpp/src/Ice/Reference.h
@@ -110,7 +110,7 @@ private:
     bool _hasContext;
     Ice::Context _context;
     std::string _facet;
-    bool _secure;
+    bool _secure; // TODO: Must be moved into RoutableReference, has no meaning for FixedReference.
 
     IceUtil::Mutex _hashMutex; // For lazy initialization of hash value.
     mutable Ice::Int _hashValue;
@@ -124,6 +124,7 @@ public:
     FixedReference(const InstancePtr&, const Ice::Identity&, const Ice::Context&, const std::string&, Mode, bool,
 	           const std::vector<Ice::ConnectionIPtr>&);
 
+    // TODO: Get rid of inline function.
     const std::vector<Ice::ConnectionIPtr>& getFixedConnections() const { return _fixedConnections; }
 
     virtual std::vector<EndpointPtr> getEndpoints() const;
diff --git a/cpp/src/Ice/RoutingTable.cpp b/cpp/src/Ice/RoutingTable.cpp
index 604bc274002..d3c73b61635 100644
--- a/cpp/src/Ice/RoutingTable.cpp
+++ b/cpp/src/Ice/RoutingTable.cpp
@@ -39,7 +39,7 @@ IceInternal::RoutingTable::add(const ObjectPrx& prx)
 	return false;
     }
 
-    ObjectPrx proxy = prx->ice_default(); // We insert the proxy in it's default form into the routing table.
+    ObjectPrx proxy = prx->ice_default(); // We insert the proxy in its default form into the routing table.
 
     IceUtil::Mutex::Lock sync(*this);