Skip to content

Commit

Permalink
Swap raw pointer for boost::shared_ptr in BinaryOperation
Browse files Browse the repository at this point in the history
The BinaryOperationTable was not deleted at the end of the operation
causing a memory leak. The shared_ptr will take care of this properly.
Refs #7994
  • Loading branch information
martyngigg committed Sep 30, 2013
1 parent 293893c commit 6f9c034
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,13 @@ namespace Mantid
* Index into vector: workspace index in the lhs;
* Value at that index: workspace index of the rhs to apply to the WI in the lhs. -1 if not found.
*/
typedef std::vector< int64_t > BinaryOperationTable;
typedef std::vector< int64_t> BinaryOperationTable;
typedef boost::shared_ptr<BinaryOperationTable> BinaryOperationTable_sptr;

static BinaryOperationTable * buildBinaryOperationTable(API::MatrixWorkspace_const_sptr lhs, API::MatrixWorkspace_const_sptr rhs);
static BinaryOperationTable_sptr
buildBinaryOperationTable(const API::MatrixWorkspace_const_sptr & lhs, const API::MatrixWorkspace_const_sptr & rhs);


//protected:
private:
// Overridden Algorithm methods
void init();
Expand Down
12 changes: 8 additions & 4 deletions Code/Mantid/Framework/Algorithms/src/BinaryOperation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "MantidKernel/Timer.h"
#include "MantidDataObjects/WorkspaceSingleValue.h"

#include <boost/make_shared.hpp>

using namespace Mantid::Geometry;
using namespace Mantid::API;
using namespace Mantid::Kernel;
Expand Down Expand Up @@ -675,10 +677,11 @@ namespace Mantid
*/
void BinaryOperation::do2D( bool mismatchedSpectra)
{
BinaryOperationTable * table = NULL;
BinaryOperationTable_sptr table;
if (mismatchedSpectra)
{
table = BinaryOperation::buildBinaryOperationTable(m_lhs, m_rhs);

}

// Propagate any masking first or it could mess up the numbers
//TODO: Check if this works for event workspaces...
Expand Down Expand Up @@ -999,12 +1002,13 @@ namespace Mantid
* @return map from detector ID to workspace index for the RHS workspace.
* NULL if there is not a 1:1 mapping from detector ID to workspace index (e.g more than one detector per pixel).
*/
BinaryOperation::BinaryOperationTable * BinaryOperation::buildBinaryOperationTable(MatrixWorkspace_const_sptr lhs, MatrixWorkspace_const_sptr rhs)
BinaryOperation::BinaryOperationTable_sptr
BinaryOperation::buildBinaryOperationTable(const MatrixWorkspace_const_sptr &lhs, const MatrixWorkspace_const_sptr & rhs)
{
//An addition table is a list of pairs:
// First int = workspace index in the EW being added
// Second int = workspace index to which it will be added in the OUTPUT EW. -1 if it should add a new entry at the end.
BinaryOperationTable * table = new BinaryOperationTable();
auto table = boost::make_shared<BinaryOperationTable>();

int rhs_nhist = static_cast<int>(rhs->getNumberHistograms());
int lhs_nhist = static_cast<int>(lhs->getNumberHistograms());
Expand Down
14 changes: 7 additions & 7 deletions Code/Mantid/Framework/Algorithms/test/BinaryOperationTest.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,12 @@ class BinaryOperationTest : public CxxTest::TestSuite
}


BinaryOperation::BinaryOperationTable * do_test_buildBinaryOperationTable(std::vector< std::vector<int> > lhs, std::vector< std::vector<int> > rhs,
BinaryOperation::BinaryOperationTable_sptr do_test_buildBinaryOperationTable(std::vector< std::vector<int> > lhs, std::vector< std::vector<int> > rhs,
bool expect_throw = false)
{
EventWorkspace_sptr lhsWS = WorkspaceCreationHelper::CreateGroupedEventWorkspace(lhs, 50, 1.0);
EventWorkspace_sptr rhsWS = WorkspaceCreationHelper::CreateGroupedEventWorkspace(rhs, 50, 1.0);
BinaryOperation::BinaryOperationTable * table = 0;
BinaryOperation::BinaryOperationTable_sptr table;
Mantid::Kernel::Timer timer1;
if (expect_throw)
{
Expand Down Expand Up @@ -206,7 +206,7 @@ class BinaryOperationTest : public CxxTest::TestSuite
// 3 detectors in each on the rhs
rhs[i/3].push_back(i);
}
BinaryOperation::BinaryOperationTable * table = do_test_buildBinaryOperationTable(lhs, rhs);
auto table = do_test_buildBinaryOperationTable(lhs, rhs);
for (int i=0; i<6; i++)
{
TS_ASSERT_EQUALS( (*table)[i], i/3);
Expand All @@ -223,7 +223,7 @@ class BinaryOperationTest : public CxxTest::TestSuite
// 3 detectors in each on the rhs
rhs[i/3].push_back(i);
}
BinaryOperation::BinaryOperationTable * table = do_test_buildBinaryOperationTable(lhs, rhs, false);
auto table = do_test_buildBinaryOperationTable(lhs, rhs, false);
TS_ASSERT_EQUALS( (*table)[0], 1);
TS_ASSERT_EQUALS( (*table)[1], 1);
TS_ASSERT_EQUALS( (*table)[2], 1);
Expand All @@ -243,7 +243,7 @@ class BinaryOperationTest : public CxxTest::TestSuite
// 4 detectors in each on the rhs
rhs[i/4].push_back(i);
}
BinaryOperation::BinaryOperationTable * table = do_test_buildBinaryOperationTable(lhs, rhs);
auto table = do_test_buildBinaryOperationTable(lhs, rhs);
for (int i=0; i<8; i++)
{
TS_ASSERT_EQUALS( (*table)[i], i/2);
Expand All @@ -260,7 +260,7 @@ class BinaryOperationTest : public CxxTest::TestSuite
// 6 detectors in each on the rhs
rhs[i/6].push_back(i);
}
BinaryOperation::BinaryOperationTable * table = do_test_buildBinaryOperationTable(lhs, rhs, false);
auto table = do_test_buildBinaryOperationTable(lhs, rhs, false);
TS_ASSERT_EQUALS( (*table)[0], 0); //0-3 go into 0-5
TS_ASSERT_EQUALS( (*table)[1], -1); //4-7 fails to go anywhere
TS_ASSERT_EQUALS( (*table)[2], 1); //8-11 goes into 6-11
Expand All @@ -277,7 +277,7 @@ class BinaryOperationTest : public CxxTest::TestSuite
// 1000 detectors in each on the rhs
rhs[i/100][i%100] = i;
}
BinaryOperation::BinaryOperationTable * table = do_test_buildBinaryOperationTable(lhs, rhs);
auto table = do_test_buildBinaryOperationTable(lhs, rhs);
for (int i=0; i<2000; i++)
{
TS_ASSERT_EQUALS( (*table)[i], i/100);
Expand Down

0 comments on commit 6f9c034

Please sign in to comment.