ChangeLogTag:Fri Mar 11 16:29:30 (IST) 2005 Balachandran Natarajan <bala@dre.vanderbilt.edu>

14 files changed, 166 insertions, 81 deletions

diff --git a/TAO/CIAO/DAnCE/ChangeLog b/TAO/CIAO/DAnCE/ChangeLog
index 38d1b26fb02..d573ee16a44 100644
--- a/TAO/CIAO/DAnCE/ChangeLog
+++ b/TAO/CIAO/DAnCE/ChangeLog
@@ -1,9 +1,28 @@
+Fri Mar 11 16:29:30 (IST) 2005  Balachandran Natarajan  <bala@dre.vanderbilt.edu>
+
+	* ciao/Container_Base.h:
+	* ciao/Container_Impl.cpp:
+	* ciao/Container_Impl.h:
+	* ciao/Dynamic_Component_Activator.cpp:
+	* ciao/Dynamic_Component_Activator.h:
+	* ciao/Dynamic_Component_Servant_Base.h:
+	* ciao/Dynamic_Component_Servant_T.cpp:
+	* ciao/Dynamic_Component_Servant_T.h:
+	* ciao/NodeApplication_Impl.cpp:
+	* ciao/NodeApplication_Impl.h:
+	* ciao/Swapping_Container.cpp:
+	* ciao/Swapping_Container.h:
+	* examples/Hello/Sender/Sender_svnt.cpp.with-connections:
+
+	  Left @@ comments for Jai and some for Gan. 
+
 Thu Mar 10 13:46:21 2005  Will Otte <wotte@dre.vanderbilt.edu>
+
 	* XMLSchema/Types.hpp
 	* XSCRT/ExtendedTypeInfo.ipp
 
-	Updated the XSCRT runtime to the latest version to fix warnings
-	in VC7.
+	  Updated the XSCRT runtime to the latest version to fix warnings
+	  in VC7. 
 	
 Thu Mar 10 10:29:32 2005  Jaiganesh B  <jai@dre.vanderbilt.edu>
 
diff --git a/TAO/CIAO/DAnCE/ciao/Container_Base.h b/TAO/CIAO/DAnCE/ciao/Container_Base.h
index c92af3cd296..e053135f6f3 100644
--- a/TAO/CIAO/DAnCE/ciao/Container_Base.h
+++ b/TAO/CIAO/DAnCE/ciao/Container_Base.h
@@ -97,6 +97,8 @@ namespace CIAO
                                       ACE_ENV_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException)) = 0;
 
+    // @@Jai, please see the Session Container class for comments.
+    // @@ Jai, do you really need the environment variable?
     virtual void add_servant_map (PortableServer::ObjectId &oid,
                                   Dynamic_Component_Servant_Base* servant
                                   ACE_ENV_ARG_DECL) = 0;
@@ -243,13 +245,26 @@ namespace CIAO
                     ACE_ENV_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException));
 
+    // @@Jai, please consider naming this method as
+    // "add_servant_to_map ()" to be more descriptive.
     virtual void add_servant_map (PortableServer::ObjectId &oid,
                                   Dynamic_Component_Servant_Base* servant
                                   ACE_ENV_ARG_DECL);
 
+    // @@Jai, please consider naming this method as
+    // "delete_servant_from_map ()" to be more descriptive.
     virtual void delete_servant_map (PortableServer::ObjectId &oid
                                      ACE_ENV_ARG_DECL);
 
+    // @@Jai, could yo please add documentation?
+    /*
+     * @@Jai, you may want to consider moving these away from the
+     * container interface. I know what you are going to say
+     * :-). Consider using dynamic_cast <> to access
+     * add_servant_to_map, delete_servant_from_map and
+     * deactivate_facet from the Swapping_Conatiner's interface. It
+     * would make the base container interface much cleaner.
+     */
     virtual void deactivate_facet (PortableServer::ObjectId &oid
                                    ACE_ENV_ARG_DECL)
       ACE_THROW_SPEC ((CORBA::SystemException));
diff --git a/TAO/CIAO/DAnCE/ciao/Container_Impl.cpp b/TAO/CIAO/DAnCE/ciao/Container_Impl.cpp
index a7b47c9a18f..0d73be12fed 100644
--- a/TAO/CIAO/DAnCE/ciao/Container_Impl.cpp
+++ b/TAO/CIAO/DAnCE/ciao/Container_Impl.cpp
@@ -24,6 +24,8 @@ CIAO::Container_Impl::init (const ::Deployment::Properties &properties
                             ACE_ENV_ARG_DECL)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
+  // @@ Gan/Jai, see how this init can create many instances of the
+  // same container? Neither thread safe or single entity safe.
   this->properties_ = properties;
 
   // @@ Initialize container and create the internal container
@@ -33,6 +35,9 @@ CIAO::Container_Impl::init (const ::Deployment::Properties &properties
   // @@ We will need a container factory here later on when we support
   // more kinds of container implementations.
 
+  // @@Jai, what is the condition to create an upgradeable container?
+  // Where is it getting created and how? Need to address that.
+
   ACE_NEW_THROW_EX (this->container_,
                     CIAO::Session_Container (this->orb_.in (), this),
                     CORBA::INTERNAL ());
@@ -74,7 +79,7 @@ CIAO::Container_Impl::install (
      for (CORBA::ULong i = 0; i < len; ++i)
        {
          // Install home
-         Components::CCMHome_var home = 
+         Components::CCMHome_var home =
            this->install_home (impl_infos[i]
                                ACE_ENV_ARG_PARAMETER);
          ACE_TRY_CHECK;
diff --git a/TAO/CIAO/DAnCE/ciao/Container_Impl.h b/TAO/CIAO/DAnCE/ciao/Container_Impl.h
index a905053697d..85666635d38 100644
--- a/TAO/CIAO/DAnCE/ciao/Container_Impl.h
+++ b/TAO/CIAO/DAnCE/ciao/Container_Impl.h
@@ -40,9 +40,9 @@ namespace CIAO
    * @brief Servant implementation for the interface Deployment::Container
    *
    * This class implements the Deployment::Container
-   * interface which is not defined by the CCM DnC specification. 
-   * As the interface implies, this is actually part of the deployment 
-   * interface and is used to manage the lifecycle of the installed 
+   * interface which is not defined by the CCM DnC specification.
+   * As the interface implies, this is actually part of the deployment
+   * interface and is used to manage the lifecycle of the installed
    * components and homes.
    */
   class CIAO_SERVER_Export Container_Impl
@@ -67,7 +67,7 @@ namespace CIAO
       ACE_THROW_SPEC ((CORBA::SystemException));
 
     /// Install all homes and components
-    Deployment::ComponentInfos * 
+    Deployment::ComponentInfos *
       install (const ::Deployment::ContainerImplementationInfo & container_impl_info
                ACE_ENV_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException,
@@ -77,17 +77,17 @@ namespace CIAO
                          Components::InvalidConfiguration));
 
     /// Remove all homes and components
-    virtual void 
+    virtual void
       remove (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException,
                          Components::RemoveFailure));
 
     /// Deployment::Container interface defined attributes/operations.
-    virtual ::Deployment::Properties * 
+    virtual ::Deployment::Properties *
       properties (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException));
 
-    virtual ::Deployment::NodeApplication_ptr 
+    virtual ::Deployment::NodeApplication_ptr
       get_node_application (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException));
 
@@ -100,7 +100,7 @@ namespace CIAO
 
 
     // Install the home of this particular component
-    virtual ::Components::CCMHome_ptr 
+    virtual ::Components::CCMHome_ptr
       install_home (const ::Deployment::ComponentImplementationInfo & impl_info
                     ACE_ENV_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException,
@@ -120,15 +120,15 @@ namespace CIAO
      * side support of this either. So bear me if you think I avoid
      * the real thinking for easiness.
      */
-    // Remove the home of this particular component 
-    virtual void 
+    // Remove the home of this particular component
+    virtual void
       remove_home (const char * comp_ins_name
                    ACE_ENV_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException,
                          Components::RemoveFailure));
 
     // Get all homes
-    virtual ::Components::CCMHomes * 
+    virtual ::Components::CCMHomes *
       get_homes (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
         ACE_THROW_SPEC ((CORBA::SystemException));
 
@@ -144,8 +144,8 @@ namespace CIAO
     virtual void remove_component (const char * comp_ins_name
                                    ACE_ENV_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException,
-                       Components::RemoveFailure));    
-    
+                       Components::RemoveFailure));
+
     /// Set the cached object reference.
     void set_objref (Deployment::Container_ptr o
                      ACE_ENV_ARG_DECL_WITH_DEFAULTS);
@@ -154,6 +154,9 @@ namespace CIAO
     /// _this if there's no cached reference available.  Notice that
     /// this method does *NOT* increase the reference count of the
     /// cached reference.
+    /// @@ Gan, caller should own memory. In this case you shoudl
+    /// return a duplicate OR add a const to the operation
+    /// signature. This is simply bad programming.
     Deployment::Container_ptr get_objref (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS);
 
 protected:
@@ -164,6 +167,7 @@ protected:
     PortableServer::POA_var poa_;
 
     /// Internal container implementation.
+    // @@ Gan/Jai, heard of auto_ptr <>?
     CIAO::Container *container_;
 
     /// Cached ConfigValues.
@@ -184,7 +188,8 @@ protected:
     typedef CCMHome_Map::iterator Home_Iterator;
     CCMHome_Map home_map_;
 
-    /// To sotre all created Component object.
+    /// To store all created Component object.
+    // @@Gan, see how this caching is duplicated..
     typedef ACE_Hash_Map_Manager_Ex<ACE_CString,
                                     Components::CCMObject_ptr,
                                     ACE_Hash<ACE_CString>,
diff --git a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.cpp b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.cpp
index c2798a31f8e..41aaa02d477 100644
--- a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.cpp
+++ b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.cpp
@@ -25,13 +25,14 @@ namespace CIAO
     ACE_THROW_SPEC ((CORBA::SystemException,
                      PortableServer::ForwardRequest))
   {
+    // @@Jai, where i sth emutex used at all?
     Dynamic_Component_Servant_Base* servant = 0;
     PortableServer::ObjectId new_oid (oid);
 
     if (this->servant_map_.find (oid, servant) == 0)
-    {
-      return servant->create (new_oid);
-    }
+      {
+        return servant->create (new_oid);
+      }
 
     return 0;
   }
@@ -48,7 +49,7 @@ namespace CIAO
   {
   }
 
-  void Dynamic_Component_Activator::add_servant_map 
+  void Dynamic_Component_Activator::add_servant_map
     (PortableServer::ObjectId &oid,
      Dynamic_Component_Servant_Base* servant
      ACE_ENV_ARG_DECL)
@@ -56,7 +57,7 @@ namespace CIAO
     this->servant_map_.bind (oid, servant);
   }
 
-  void Dynamic_Component_Activator::delete_servant_map 
+  void Dynamic_Component_Activator::delete_servant_map
     (PortableServer::ObjectId &oid
      ACE_ENV_ARG_DECL)
   {
diff --git a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.h b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.h
index 5e1e86c2bfc..45965c74dd0 100644
--- a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.h
+++ b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Activator.h
@@ -89,6 +89,8 @@ namespace CIAO
     /// Pointer to our ORB
     CORBA::ORB_var orb_;
 
+    // @@ Jai, why are there two mutexes here?Can you get one of them
+    //out of the way?
     /// Mutex that synchronizes access to the array.
     ACE_SYNCH_MUTEX  mutex_;
 
diff --git a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_Base.h b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_Base.h
index fff68f5965d..aaf04ee755b 100644
--- a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_Base.h
+++ b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_Base.h
@@ -15,6 +15,7 @@
 
 #include /**/ "ace/pre.h"
 
+// @@ Jai, why is this includion necessary. Looks heavy weight to me.
 #include "CCM_ComponentS.h"
 
 #if !defined (ACE_LACKS_PRAGMA_ONCE)
@@ -36,11 +37,11 @@ namespace CIAO
 
     virtual ~Dynamic_Component_Servant_Base (void);
 
-    virtual PortableServer::Servant 
+    virtual PortableServer::Servant
       create (PortableServer::ObjectId &oid) = 0;
 
     virtual void destroy (PortableServer::ObjectId &oid) = 0;
-  
+
     virtual void update_destroy_count () = 0;
 
   protected:
diff --git a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.cpp b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.cpp
index 6d44811b457..13ae100380f 100644
--- a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.cpp
+++ b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.cpp
@@ -13,18 +13,18 @@ namespace CIAO
             typename EXEC,
             typename EXEC_VAR,
             typename COMP>
-  Dynamic_Component_Servant<COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR, 
+  Dynamic_Component_Servant<COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR,
                             EXEC, EXEC_VAR, COMP>
-    ::Dynamic_Component_Servant 
+    ::Dynamic_Component_Servant
         (Components::EnterpriseComponent_ptr ec,
          Components::CCMHome_ptr home,
          Home_Servant_Impl_Base *home_servant,
          Session_Container *c)
-      :Dynamic_Component_Servant_Base (c),
-       executor_ (Components::EnterpriseComponent::_duplicate (ec)),
-       home_servant_ (home_servant),
-       home_ (Components::CCMHome::_duplicate (home)),
-       component_removed_ (0)
+      : Dynamic_Component_Servant_Base (c)
+      , executor_ (Components::EnterpriseComponent::_duplicate (ec))
+      , home_servant_ (home_servant)
+      , home_ (Components::CCMHome::_duplicate (home))
+      , component_removed_ (false)
   {
   }
 
@@ -37,8 +37,8 @@ namespace CIAO
   void Dynamic_Component_Servant<COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR,
     EXEC, EXEC_VAR, COMP>::update_destroy_count ()
   {
-    component_removed_ = 1;
-  } 
+    component_removed_ = true;
+  }
 
   template <typename COMP_SVNT,
             typename COMP_EXEC,
@@ -49,7 +49,7 @@ namespace CIAO
   Dynamic_Component_Servant<COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR,
     EXEC, EXEC_VAR, COMP>::~Dynamic_Component_Servant (void)
   {
-  } 
+  }
 
   template <typename COMP_SVNT,
             typename COMP_EXEC,
@@ -58,14 +58,18 @@ namespace CIAO
             typename EXEC_VAR,
             typename COMP>
   void Dynamic_Component_Servant
-    <COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR, 
+    <COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR,
      EXEC, EXEC_VAR, COMP>::destroy (PortableServer::ObjectId &oid)
   {
+    // @@Jai, here is a logical problem. You find a servant from a map
+    // having a list of servants and you turn on a bit. What is the
+    // granularity of the bit? Looks like its too coarse. Do you see
+    // what I am saying?
     COMP_SVNT *servant;
     if (this->servant_map_.find (oid, servant) == 0)
       {
         servant->remove ();
-        component_removed_ = 1;
+        component_removed_ = true;
       }
   }
 
@@ -76,31 +80,35 @@ namespace CIAO
             typename EXEC_VAR,
             typename COMP>
   PortableServer::Servant Dynamic_Component_Servant
-    <COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR, 
+    <COMP_SVNT, COMP_EXEC, COMP_EXEC_VAR,
      EXEC, EXEC_VAR, COMP>::create (PortableServer::ObjectId &oid)
   {
-    CIAO::Swap_Exec_var swap_exec = CIAO::Swap_Exec::_narrow
-       (this->executor_.in ()
-        ACE_ENV_ARG_PARAMETER);
+    CIAO::Swap_Exec_var swap_exec =
+      CIAO::Swap_Exec::_narrow (this->executor_.in ()
+                                ACE_ENV_ARG_PARAMETER);
+    ACE_CHECK_RETURN (COMP::_nil ());
 
     ::Components::EnterpriseComponent_var ciao_ec =
       swap_exec->incarnate (ACE_ENV_SINGLE_ARG_PARAMETER);
     ACE_CHECK_RETURN (COMP::_nil ());
 
-    COMP_EXEC_VAR ciao_comp = COMP_EXEC::_narrow (ciao_ec.in ()
-                                                  ACE_ENV_ARG_PARAMETER);
+    COMP_EXEC_VAR ciao_comp =
+      COMP_EXEC::_narrow (ciao_ec.in ()
+                          ACE_ENV_ARG_PARAMETER);
     ACE_CHECK_RETURN (COMP::_nil ());
-    
+
+    /// @@ Jai, should you be using ACE_NEW_THROW_EX here?
     COMP_SVNT *svt = new COMP_SVNT(ciao_comp.in (), this->home_.in (),
                                    this->home_servant_,
                                    this->container_);
 
-    if (component_removed_ == 1)
+    // @@Jai, see the problem with a coarse grained stuff here.
+    if (component_removed_ == true)
       {
         // svt->ciao_preactivate ();
         // svt->ciao_activate ();
         // svt->ciao_postactivate ();
-        component_removed_ = 0;
+        component_removed_ = false;
       }
 
     PortableServer::ServantBase_var safe (svt);
diff --git a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.h b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.h
index 9e4c8828fc8..38bc9322314 100644
--- a/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.h
+++ b/TAO/CIAO/DAnCE/ciao/Dynamic_Component_Servant_T.h
@@ -30,13 +30,14 @@
 
 namespace CIAO
 {
-  template <typename COMP_SVNT, typename COMP_EXEC, 
+  template <typename COMP_SVNT, typename COMP_EXEC,
             typename COMP_EXEC_VAR, typename EXEC, typename EXEC_VAR,
             typename COMP>
   class Dynamic_Component_Servant
     : public virtual Dynamic_Component_Servant_Base
   {
   public:
+    // @@Jai, please add documentation for these methods.
     Dynamic_Component_Servant (Components::EnterpriseComponent_ptr ec,
                                Components::CCMHome_ptr home,
                                Home_Servant_Impl_Base *home_servant,
@@ -44,24 +45,30 @@ namespace CIAO
 
     virtual ~Dynamic_Component_Servant (void);
 
-    virtual PortableServer::Servant 
+    virtual PortableServer::Servant
       create (PortableServer::ObjectId &oid);
 
     virtual void destroy (PortableServer::ObjectId &oid);
 
-    virtual void update_destroy_count ();
+    virtual void update_destroy_count (void);
 
   protected:
     Components::EnterpriseComponent_var executor_;
+
     Home_Servant_Impl_Base *home_servant_;
+
     Components::CCMHome_var home_;
+
+    // @@ Jai, please document why this is there.
     ACE_Hash_Map_Manager_Ex<PortableServer::ObjectId,
                             COMP_SVNT *,
                             TAO_ObjectId_Hash,
                             ACE_Equal_To<PortableServer::ObjectId>,
                             ACE_SYNCH_MUTEX>
       servant_map_;
-    int component_removed_;
+
+    // @@ Jai, please explain what this flag means?
+    bool component_removed_;
   };
 }
 
diff --git a/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.cpp b/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.cpp
index 7f519450610..fa514132dda 100644
--- a/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.cpp
+++ b/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.cpp
@@ -30,7 +30,7 @@ CIAO::NodeApplication_Impl::create_all_containers (
   const CORBA::ULong len = node_impl_info.length ();
   for (CORBA::ULong i = 0; i < len; ++i)
   {
-    // The factory method <create_container> will intialize the container 
+    // The factory method <create_container> will intialize the container
     // servant with properties, so we don't need to call <init> on the
     // container object reference.
     // Also, the factory method will add the container object reference
@@ -262,7 +262,7 @@ CIAO::NodeApplication_Impl::remove (ACE_ENV_SINGLE_ARG_DECL)
 
 
 // Create a container interface, which will be hosted in this NodeApplication.
-::Deployment::Container_ptr 
+::Deployment::Container_ptr
 CIAO::NodeApplication_Impl::create_container (const ::Deployment::Properties &properties
                                               ACE_ENV_ARG_DECL)
   ACE_THROW_SPEC ((CORBA::SystemException,
@@ -277,15 +277,16 @@ CIAO::NodeApplication_Impl::create_container (const ::Deployment::Properties &pr
                     CIAO::Container_Impl (this->orb_.in (),
                                           this->poa_.in (),
                                           this->get_objref ()),
-                    CORBA::INTERNAL ());
+                    CORBA::NO_MEMORY ());
   ACE_CHECK_RETURN (0);
 
   PortableServer::ServantBase_var safe_servant (container_servant);
 
-  // @TODO: Need to decide a "component_installation" equivalent data structure
-  // to pass to the container, which will be used to suggest how to install the components.
-  // Each such data stucture should be correspond to one <process_collocation> tag
-  // in the XML file to describe the deployment plan.
+  // @TODO: Need to decide a "component_installation" equivalent data
+  // structure  to pass to the container, which will be used to
+  // suggest how to install the components.  Each such data stucture
+  // should be correspond to one <process_collocation> tag  in the XML
+  // file to describe the deployment plan.
   container_servant->init (properties
                            ACE_ENV_ARG_PARAMETER);
   ACE_CHECK_RETURN (0);
@@ -315,7 +316,8 @@ CIAO::NodeApplication_Impl::create_container (const ::Deployment::Properties &pr
     this->container_set_.add (ci.in ());
   }
 
-  ACE_DEBUG ((LM_DEBUG, "LEAVING: NodeApplication_Impl::create_container()\n"));
+  ACE_DEBUG ((LM_DEBUG,
+              "LEAVING: NodeApplication_Impl::create_container()\n"));
   return ci._retn ();
 }
 
@@ -353,7 +355,7 @@ CIAO::NodeApplication_Impl::remove_container (::Deployment::Container_ptr cref
 }
 
 // Get containers
-::Deployment::Containers * 
+::Deployment::Containers *
 CIAO::NodeApplication_Impl::get_containers (ACE_ENV_SINGLE_ARG_DECL)
   ACE_THROW_SPEC ((CORBA::SystemException))
 {
diff --git a/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.h b/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.h
index 3854df01d27..4e021655f21 100644
--- a/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.h
+++ b/TAO/CIAO/DAnCE/ciao/NodeApplication_Impl.h
@@ -46,10 +46,16 @@ using CIAO::Utility::write_IOR;
  *
  * @@Assumptions:
  * 1. Now the implementation is not thread safe.
+ * // @@Gan, the above assumption is _really_ bad. Could you please
+ * use the lock in the imeplementation to do some simple
+ * prootections.
  **/
 
 namespace CIAO
 {
+  // @@ Gan, as we discussed before can you please wrap this
+  // implementation in a namespace Node_Application or whatever to
+  // signify that it belongs to another software piece of CIAO?
   class CIAO_SERVER_Export NodeApplication_Impl
     : public virtual POA_Deployment::NodeApplication,
       public virtual PortableServer::RefCountServantBase
@@ -107,24 +113,24 @@ namespace CIAO
                        ::Deployment::ImplEntryPointNotFound,
                        ::Deployment::InstallationFailure,
                        ::Components::InvalidConfiguration));
-                       
+
     /// Get the object reference of the NodeApplicationManager.
     /// This might come in handy later.
     virtual ::CORBA::Object_ptr
     get_node_application_manager (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException));
 
-    // Access the readonly attribute.
+    /// Access the readonly attribute.
     virtual ::Deployment::Properties *
     properties (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException));
 
-    // Remove everything inside including all components and homes.
+    /// Remove everything inside including all components and homes.
     virtual void remove (ACE_ENV_SINGLE_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException));
-      
+
     /// Create a container interface, which will be hosted in this NodeApplication.
-    virtual ::Deployment::Container_ptr 
+    virtual ::Deployment::Container_ptr
       create_container (const ::Deployment::Properties &properties
                         ACE_ENV_ARG_DECL_WITH_DEFAULTS)
       ACE_THROW_SPEC ((CORBA::SystemException,
@@ -175,7 +181,11 @@ namespace CIAO
       ACE_THROW_SPEC ((CORBA::SystemException,
                        Deployment::StartError));
 
-    /// To sotre all created Component object.
+    /// To store all created Component object.
+    // @@Gan/Jai, as we discussed before this is simply a BAD
+    //idea. These need to moved into the container.
+    // @@ Jai/Gan, how about using CCMObject_var instead of
+    //CCMObject_ptr's?
     typedef ACE_Hash_Map_Manager_Ex<ACE_CString,
                                     Components::CCMObject_ptr,
                                     ACE_Hash<ACE_CString>,
@@ -189,11 +199,13 @@ namespace CIAO
 
     /// Keep a list of managed Container objects.
     Object_Set<Deployment::Container, Deployment::Container_var> container_set_;
-
     /// Keep a pointer to the managing ORB serving this servant.
     CORBA::ORB_var orb_;
 
     /// Keep a pointer to the managing POA.
+    // @@Gan/Jai, which POA is this? Same as the component POA or a
+    // different one. My sense is that its different. Could you please
+    //document it?
     PortableServer::POA_var poa_;
 
     /// Cached properties
diff --git a/TAO/CIAO/DAnCE/ciao/Swapping_Container.cpp b/TAO/CIAO/DAnCE/ciao/Swapping_Container.cpp
index 1cff188022f..5e33f1ca911 100644
--- a/TAO/CIAO/DAnCE/ciao/Swapping_Container.cpp
+++ b/TAO/CIAO/DAnCE/ciao/Swapping_Container.cpp
@@ -105,7 +105,7 @@ namespace CIAO
     ACE_CHECK_RETURN (-1);
 
     this->create_connections_POA (root_poa.in ()
-                           ACE_ENV_ARG_PARAMETER);
+                                  ACE_ENV_ARG_PARAMETER);
     ACE_CHECK_RETURN (-1);
 
     PortableServer::POAManager_var poa_manager =
@@ -213,6 +213,9 @@ namespace CIAO
                                           PortableServer::POA_ptr root
                                           ACE_ENV_ARG_DECL)
   {
+    // @@ Jai, see how this method more or less does things (like
+    // setting policies)  like create_connections_POA (). Could you
+    // please refactor them into a seperate method?
     CORBA::PolicyList policies (0);
 
     if (p != 0)
@@ -286,9 +289,9 @@ namespace CIAO
     PortableServer::POA_ptr tmp = 0;
 
     if (t == Container::Component)
-    {
-      tmp = this->home_servant_poa_.in ();
-    }
+      {
+        tmp = this->home_servant_poa_.in ();
+      }
     else
       tmp = this->facet_cons_poa_.in ();
 
@@ -296,7 +299,7 @@ namespace CIAO
       = tmp->activate_object (p
                               ACE_ENV_ARG_PARAMETER);
     ACE_CHECK_RETURN (0);
-    
+
     CORBA::Object_var objref
       = tmp->id_to_reference (oid.in ()
                               ACE_ENV_ARG_PARAMETER);
diff --git a/TAO/CIAO/DAnCE/ciao/Swapping_Container.h b/TAO/CIAO/DAnCE/ciao/Swapping_Container.h
index 7d8bef97c21..728c3e3dd16 100644
--- a/TAO/CIAO/DAnCE/ciao/Swapping_Container.h
+++ b/TAO/CIAO/DAnCE/ciao/Swapping_Container.h
@@ -18,6 +18,8 @@
 #define CIAO_SWAPPING_CONTAINER_H
 #include /**/ "ace/pre.h"
 
+// @@ Jai, are all these inclusions necessary? Please cut down on them
+// to a minimum
 #include "tao/ORB.h"
 #include "tao/PortableServer/PortableServer.h"
 #include "tao/PortableServer/Servant_Base.h"
@@ -113,6 +115,7 @@ namespace CIAO
 // tao/ValueType/ValueFactory.h but here we take advantage of
 // the fact that we have access to the current ORB indirectly
 // through the context and container.
+// @@Jai, is this macro required here?
 #define CIAO_REGISTER_OBV_FACTORY(FACTORY, VALUETYPE) \
   {  \
     CORBA::ValueFactory factory = new FACTORY; \
diff --git a/TAO/CIAO/DAnCE/examples/Hello/Sender/Sender_svnt.cpp.with-connections b/TAO/CIAO/DAnCE/examples/Hello/Sender/Sender_svnt.cpp.with-connections
index 2f25588a3f5..8edad48a892 100644
--- a/TAO/CIAO/DAnCE/examples/Hello/Sender/Sender_svnt.cpp.with-connections
+++ b/TAO/CIAO/DAnCE/examples/Hello/Sender/Sender_svnt.cpp.with-connections
@@ -92,7 +92,7 @@ namespace Sender_Impl
     ::Components::CCMHome_ptr home,
     ::CIAO::Session_Container *c,
     Sender_Servant *sv)
-      : ACE_NESTED_CLASS (CIAO, Context_Impl_Base (home, c)), 
+      : ACE_NESTED_CLASS (CIAO, Context_Impl_Base (home, c)),
       ctx_svnt_base (home, c, sv)
     {
     }
@@ -112,7 +112,7 @@ namespace Sender_Impl
     {
       PortableServer::ObjectId_var oid =
         PortableServer::string_to_ObjectId (obj_id);
-      CIAO::Servant_Activator *sa = 
+      CIAO::Servant_Activator *sa =
       this->container_->ports_servant_activator ();
       sa->update_port_activator (oid);
     }
@@ -146,17 +146,17 @@ namespace Sender_Impl
       retval->length (this->ciao_publishes_click_out_map_.current_size ());
       CORBA::ULong i = 0;
 
-      ACE_Active_Map_Manager< 
+      ACE_Active_Map_Manager<
       ::Hello::TimeOutConsumer_var>::iterator end =
       this->ciao_publishes_click_out_map_.end ();
 
-      for (ACE_Active_Map_Manager< 
+      for (ACE_Active_Map_Manager<
       ::Hello::TimeOutConsumer_var>::iterator iter =
       this->ciao_publishes_click_out_map_.begin ();
       iter != end;
       ++iter)
       {
-        ACE_Active_Map_Manager< 
+        ACE_Active_Map_Manager<
         ::Hello::TimeOutConsumer_var>::ENTRY &entry = *iter;
 
         ::Hello::TimeOutConsumer_var c =
@@ -186,17 +186,17 @@ namespace Sender_Impl
     ACE_ENV_ARG_DECL)
     ACE_THROW_SPEC ((CORBA::SystemException))
     {
-      ACE_Active_Map_Manager< 
+      ACE_Active_Map_Manager<
       ::Hello::TimeOutConsumer_var>::iterator end =
       this->ciao_publishes_click_out_map_.end ();
 
-      for (ACE_Active_Map_Manager< 
+      for (ACE_Active_Map_Manager<
       ::Hello::TimeOutConsumer_var>::iterator iter =
       this->ciao_publishes_click_out_map_.begin ();
       iter != end;
       ++iter)
       {
-        ACE_Active_Map_Manager< 
+        ACE_Active_Map_Manager<
         ::Hello::TimeOutConsumer_var>::ENTRY &entry = *iter;
 
         ::Hello::TimeOutConsumer_var c =
@@ -398,7 +398,7 @@ namespace Sender_Impl
         >
       MACRO_MADNESS_TYPEDEF;
 
-      ACE_NEW_THROW_EX ( 
+      ACE_NEW_THROW_EX (
       tmp,
       MACRO_MADNESS_TYPEDEF (
       "Hello_Sender_push_message",
@@ -409,7 +409,7 @@ namespace Sender_Impl
       this),
       CORBA::NO_MEMORY ());
 
-      CIAO::Servant_Activator *sa = 
+      CIAO::Servant_Activator *sa =
       this->container_->ports_servant_activator ();
 
       if (!sa->register_port_activator (tmp))
@@ -635,6 +635,7 @@ namespace Sender_Impl
     ACE_ENV_SINGLE_ARG_DECL)
     ACE_THROW_SPEC ((CORBA::SystemException))
     {
+      // @@Jai, an ACE_CHECK please
       this->activate_component ();
       this->executor_->start (
       ACE_ENV_SINGLE_ARG_PARAMETER);
@@ -647,6 +648,7 @@ namespace Sender_Impl
     ACE_ENV_SINGLE_ARG_DECL)
     ACE_THROW_SPEC ((CORBA::SystemException))
     {
+      // @@Jai, an ACE_CHECK_RETURN please
       this->activate_component ();
       return this->executor_->local_message (
       ACE_ENV_SINGLE_ARG_PARAMETER);
@@ -670,6 +672,7 @@ namespace Sender_Impl
     ACE_ENV_SINGLE_ARG_DECL)
     ACE_THROW_SPEC ((CORBA::SystemException))
     {
+      // @@ Jai, Why is this required?
       ACE_ENV_ARG_NOT_USED;
       ::CORBA::Object_var obj_var;
       ::Components::EventConsumerBase_var ecb_var;
@@ -733,4 +736,3 @@ namespace Sender_Impl
     c);
   }
 }
-