BugID: 991 956

Merged changes on task branch for Bug #956 with the main development line.

CEP/BB/BBSControl/include/BBSControl/BBSMultiStep.h

@@ -88,7 +88,8 @@ namespace LOFAR
       // Implementation of getAllSteps() for BBSMultiStep. It retrieves all
       // steps by calling getAllSteps() on all steps that comprise this
       // multistep.
-      virtual void doGetAllSteps(vector<const BBSStep*>& steps) const;
+      virtual void 
+      doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const;
 
       // Check to see if there's an infinite recursion present in the
       // definition of a BBSMultiStep. This can happen when one of the steps
@@ -97,7 +98,7 @@ namespace LOFAR
       void infiniteRecursionCheck(const string& name) const;
 
       // Vector holding a sequence of BBSSteps.
-      vector<const BBSStep*> itsSteps;
+      vector< shared_ptr<const BBSStep> > itsSteps;
     };
 
     // @}

CEP/BB/BBSControl/include/BBSControl/BBSStep.h

@@ -33,6 +33,7 @@
 #include <Common/lofar_vector.h>
 #include <Common/lofar_iosfwd.h>
 #include <Common/LofarTypes.h>
+#include <Common/lofar_smartptr.h>
 
 namespace LOFAR
 {
@@ -61,7 +62,8 @@ namespace LOFAR
     // cause the pointer to never be freed. This can happen since a BBSStep
     // stores a pointer to its parent as backreference. Here, we should
     // probably use a boost::weak_ptr. See Bug #906
-    class BBSStep : public Command
+    class BBSStep : public Command,
+                    public enable_shared_from_this<BBSStep>
     {
     public:
       // Destructor.
@@ -80,7 +82,7 @@ namespace LOFAR
       // class that can be used to iterate over the all steps. I had some
       // trouble getting that thingy working, so, due to time constraints, I
       // implemented things the ugly way.
-      vector<const BBSStep*> getAllSteps() const;
+      vector< shared_ptr<const BBSStep> > getAllSteps() const;
 
       // Create a new step object. The new step can either be a BBSSingleStep
       // or a BBSMultiStep object. This is determined by examining the
@@ -88,9 +90,9 @@ namespace LOFAR
       // <tt>Step.<em>name</em>.Steps</tt>, then \a aName is a BBSMultiStep,
       // otherwise it is a SingleStep. The third, optional, argument is used
       // to pass a backreference to the parent BBSStep object.
-      static BBSStep* create(const string& name,
-			     const ACC::APS::ParameterSet& parSet,
-			     const BBSStep* parent = 0);
+      static shared_ptr<BBSStep> create(const string& name,
+                                        const ACC::APS::ParameterSet& parSet,
+                                        const BBSStep* parent = 0);
 
       // Print the contents of \c *this in human readable form into the output
       // stream \a os.
@@ -153,7 +155,8 @@ namespace LOFAR
       // Implementation of getAllSteps(). The default implementation adds \c
       // this to the vector \a steps.
       // \note This method must be overridden by BBSMultiStep.
-      virtual void doGetAllSteps(vector<const BBSStep*>& steps) const;
+      virtual void
+      doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const;
 
       // Name of this step.
       string                 itsName;
@@ -192,16 +195,7 @@ namespace LOFAR
 
     };
 
-    // Factory that can be used to generate new BBSStep objects.
-    // The factory is defined as a singleton.
-    typedef Singleton < 
-      ObjectFactory < BBSStep(const string&, 
-                              const ACC::APS::ParameterSet&, 
-                              const BBSStep*), 
-                      string >
-    > BBSStepFactory;
-
-    
+   
   } // namespace BBS
 
 } // namespace LOFAR

CEP/BB/BBSControl/include/BBSControl/BBSStrategy.h

@@ -32,6 +32,7 @@
 #include <Common/lofar_iosfwd.h>
 #include <Common/lofar_string.h>
 #include <Common/lofar_vector.h>
+#include <Common/lofar_smartptr.h>
 
 namespace LOFAR
 {
@@ -79,7 +80,7 @@ namespace LOFAR
       // done in pre-order, depth-first.
       // \todo Do we really want to implement such "iterator-like behaviour"
       // in this class?
-      vector<const BBSStep*> getAllSteps() const;
+      vector< shared_ptr<const BBSStep> > getAllSteps() const;
 
       // Indicate whether the BBSSteps contained in \c itsSteps should also be
       // written when write(ParameterSet&) is called.
@@ -140,7 +141,7 @@ namespace LOFAR
       Integration            itsIntegration;
 
       // Sequence of steps that comprise this solve strategy.
-      vector<const BBSStep*> itsSteps;
+      vector< shared_ptr<const BBSStep> > itsSteps;
 
       // Flag indicating whether the BBSStep objects in \c itsSteps should
       // also be written when write(ParameterSet&) is called.

CEP/BB/BBSControl/include/BBSControl/CommandQueue.h

@@ -189,7 +189,6 @@ namespace LOFAR
       //
       // \todo Wrap multiple queries (needed for, e.g., reconstructing a
       // BBSSolveStep) in one transaction.
-//       const Command* getNextCommand();
       pair<shared_ptr<const Command>, const CommandId> getNextCommand() const;
 
       // Set the BBSStrategy in the command queue. All information, \e except

CEP/BB/BBSControl/include/BBSControl/GlobalProcessControl.h

@@ -28,6 +28,7 @@
 
 //# Includes
 #include <PLC/ProcessControl.h>
+#include <Common/lofar_smartptr.h>
 
 #include <BBSControl/CommandId.h>
 #include <BBSControl/CommandResult.h>
@@ -120,13 +121,13 @@ namespace LOFAR
 
       // Vector containing all the separate steps, in sequential order, that
       // the strategy consists of.
-      vector<const BBSStep*>  itsSteps;
+      vector< shared_ptr<const BBSStep> >  itsSteps;
 
       // Iterator for keeping track where we left while traversing the vector
       // \c itsSteps. We need this iterator, because the run() method will be
       // invoked several times by ACCmain. In each call to run() we must
       // execute one BBSStep.
-      vector<const BBSStep*>::const_iterator itsStepsIterator;
+      vector< shared_ptr<const BBSStep> >::const_iterator itsStepsIterator;
 
       // CommandQueue where strategies and steps can be "posted".
       scoped_ptr<CommandQueue> itsCommandQueue;

CEP/BB/BBSControl/src/BBSMultiStep.cc

@@ -59,12 +59,6 @@ namespace LOFAR
     BBSMultiStep::~BBSMultiStep()
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
-
-      // Clean up all steps.
-      for(uint i = 0; i < itsSteps.size(); ++i) {
-	delete itsSteps[i];
-      }
-      itsSteps.clear();
     }
 
 
@@ -139,11 +133,12 @@ namespace LOFAR
     }
 
 
-    void BBSMultiStep::doGetAllSteps(vector<const BBSStep*>& steps) const
+    void
+    BBSMultiStep::doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
       for (uint i = 0; i < itsSteps.size(); ++i) {
-	vector<const BBSStep*> substeps = itsSteps[i]->getAllSteps();
+	vector< shared_ptr<const BBSStep> > substeps = itsSteps[i]->getAllSteps();
 	steps.insert(steps.end(), substeps.begin(), substeps.end());
       }
     }

CEP/BB/BBSControl/src/BBSStep.cc

@@ -63,26 +63,27 @@ namespace LOFAR
     }
 
 
-    vector<const BBSStep*> BBSStep::getAllSteps() const
+    vector< shared_ptr<const BBSStep> > BBSStep::getAllSteps() const
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
-      vector<const BBSStep*> steps;
+      vector< shared_ptr<const BBSStep> > steps;
       doGetAllSteps(steps);
       return steps;
     }
 
 
-    BBSStep* BBSStep::create(const string& name,
-			     const ParameterSet& parset,
-			     const BBSStep* parent)
+    shared_ptr<BBSStep> BBSStep::create(const string& name,
+                                        const ParameterSet& parset,
+                                        const BBSStep* parent)
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
+      shared_ptr<BBSStep> step;
 
       // If \a parset contains a key <tt>Step.<em>name</em>.Steps</tt>, then
       // \a name is a BBSMultiStep, otherwise it is a SingleStep.
       if (parset.isDefined("Step." + name + ".Steps")) {
 	LOG_TRACE_COND_STR(name << " is a MultiStep");
-	return new BBSMultiStep(name, parset, parent);
+	step.reset(new BBSMultiStep(name, parset, parent));
       } else {
 	LOG_TRACE_COND_STR(name << " is a SingleStep");
 	// We'll have to figure out what kind of SingleStep we must
@@ -92,23 +93,24 @@ namespace LOFAR
             toUpper(parset.getString("Step." + name + ".Operation"));
           LOG_TRACE_COND_STR("Creating a " << oper << " step ...");
           if      (oper == "SOLVE")
-            return new BBSSolveStep(name, parset, parent);
+            step.reset(new BBSSolveStep(name, parset, parent));
           else if (oper == "SUBTRACT")
-            return new BBSSubtractStep(name, parset, parent);
+            step.reset(new BBSSubtractStep(name, parset, parent));
           else if (oper == "CORRECT")
-            return new BBSCorrectStep(name, parset, parent);
+            step.reset(new BBSCorrectStep(name, parset, parent));
           else if (oper == "PREDICT")
-            return new BBSPredictStep(name, parset, parent);
+            step.reset(new BBSPredictStep(name, parset, parent));
           else if (oper == "SHIFT")
-            return new BBSShiftStep(name, parset, parent);
+            step.reset(new BBSShiftStep(name, parset, parent));
           else if (oper == "REFIT")
-            return new BBSRefitStep(name, parset, parent);
+            step.reset(new BBSRefitStep(name, parset, parent));
           else THROW (BBSControlException, "Operation \"" << oper << 
                       "\" is not a valid Step operation");
         } catch (APSException& e) {
           THROW (BBSControlException, e.what());
         }
       }
+      return step;
     }
 
 
@@ -213,10 +215,11 @@ namespace LOFAR
 
     //##--------   P r i v a t e   m e t h o d s   --------##//
 
-    void BBSStep::doGetAllSteps(vector<const BBSStep*>& steps) const
+    void 
+    BBSStep::doGetAllSteps(vector< shared_ptr<const BBSStep> >& steps) const
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
-      steps.push_back(this);
+      steps.push_back(shared_from_this());
     }
     
 

CEP/BB/BBSControl/src/BBSStrategy.cc

@@ -113,12 +113,6 @@ namespace LOFAR
     BBSStrategy::~BBSStrategy()
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
-
-      // Clean up all steps.
-      for (uint i = 0; i < itsSteps.size(); ++i) {
-        delete itsSteps[i];
-      }
-      itsSteps.clear();
     }
 
 
@@ -147,12 +141,13 @@ namespace LOFAR
     }
  
 
-    vector<const BBSStep*> BBSStrategy::getAllSteps() const
+    vector< shared_ptr<const BBSStep> > BBSStrategy::getAllSteps() const
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
-      vector<const BBSStep*> steps;
+      vector< shared_ptr<const BBSStep> > steps;
       for (uint i = 0; i < itsSteps.size(); ++i) {
-	vector<const BBSStep*> substeps = itsSteps[i]->getAllSteps();
+	vector< shared_ptr<const BBSStep> > substeps =
+          itsSteps[i]->getAllSteps();
 	steps.insert(steps.end(), substeps.begin(), substeps.end());
       }
       return steps;

CEP/BB/BBSControl/src/CommandQueue.cc

@@ -24,23 +24,18 @@
 #include <lofar_config.h>
 
 //# Includes
-#include <BBSControl/CommandQueue.h>
+#include <BBSControl/Command.h>
 #include <BBSControl/CommandId.h>
+#include <BBSControl/CommandQueue.h>
 #include <BBSControl/CommandResult.h>
 #include <BBSControl/LocalControlId.h>
-// #include <BBSControl/CommandQueueTransactors.h>
-// #include <BBSControl/CommandQueueTrigger.h>
 #include <BBSControl/QueryBuilder/AddCommand.h>
-#include <BBSControl/BBSSingleStep.h>
-#include <BBSControl/BBSSolveStep.h>
+#include <BBSControl/BBSStep.h>
 #include <BBSControl/BBSStrategy.h>
-#include <BBSControl/BBSStructs.h>
 #include <BBSControl/Exceptions.h>
 #include <APS/ParameterSet.h>
 #include <APS/Exceptions.h>
-#include <Common/StreamUtil.h>
 #include <Common/LofarLogger.h>
-#include <Common/lofar_typeinfo.h>
 
 //# Now here's an ugly kludge: libpqxx defines four different top-level
 //# exception classes. In order to avoid a lot of code duplication we clumped
@@ -136,9 +131,6 @@ namespace LOFAR
     {
       LOG_TRACE_LIFETIME(TRACE_LEVEL_COND, "");
 
-      Command* command(0);
-      CommandId id;
-
       // Get the next command from the command queue. This call will only
       // return the command type.
       ParameterSet ps = 
@@ -147,10 +139,10 @@ namespace LOFAR
       // If command type is an empty string, then we've probably received an
       // empty result row; return a null pointer.
       string type = ps.getString("Type");
-      if (type.empty()) return make_pair(command, id);
+      if (type.empty()) return make_pair(shared_ptr<Command>(), CommandId());
 
       // Get the command-id
-      id = ps.getInt32("id");
+      CommandId id = ps.getInt32("id");
 
       // Retrieve the command parameters associated with the received command.
       ostringstream query;
@@ -173,13 +165,12 @@ namespace LOFAR
         ps.adoptBuffer(buf, "Step." + name + ".");
 
         // Create a new BBSStep and return it.
-        command = BBSStep::create(name, ps, 0);
-        return make_pair(command, id);
+        return make_pair(BBSStep::create(name, ps, 0), id);
       } 
       catch (APSException&) {
         // In the catch clause we handle all other commands. They can be
         // constructed using the CommandFactory and initialized using read().
-        command = CommandFactory::instance().create(type);
+        shared_ptr<Command> command(CommandFactory::instance().create(type));
         ASSERTSTR(command, "Failed to create a `" << type << 
                   "' command object");
         command->read(ps);

CEP/BB/BBSControl/test/tBBSStrategy.cc

@@ -57,6 +57,10 @@ int main()
     BBSStrategy writtenStrategy(ps);
     writtenStrategy.shouldWriteSteps(true);
 
+    {
+      BBSStrategy dummy(writtenStrategy);
+    }
+
     ps.clear();
     cout << writtenStrategy << endl;
     ps << writtenStrategy;

CEP/BB/BBSControl/test/tCommandQueue.cc

@@ -65,7 +65,7 @@ int main(int /*argc*/, char* argv[])
 //     CommandQueue queue(getenv("USER"));
     CommandQueue queue("bbs");
     CommandQueue::Trigger insert_trig(queue, CommandQueue::Trigger::Command);
-    vector<const BBSStep*> steps = strategy.getAllSteps();
+    vector< shared_ptr<const BBSStep> > steps = strategy.getAllSteps();
     ofstream ofs;
 
     // Create data; store into command queue and write to reference file.