CFD Online Discussion Forums

CFD Online Discussion Forums (http://www.cfd-online.com/Forums/)
-   OpenFOAM Bugs (http://www.cfd-online.com/Forums/openfoam-bugs/)
-   -   Problem with copy Assignment operator of Matrix (http://www.cfd-online.com/Forums/openfoam-bugs/62539-problem-copy-assignment-operator-matrix.html)

su_junwei October 15, 2007 23:16

Dear everyone I have encou
 
Dear everyone

I have encountered a segmentation fault with the class of Matrix<t>. Is it a bug, or any other reason?


file: src/OpenFOAM/containers/Matrix/Matrix.C

template<class>
void Matrix<t>::operator=(const Matrix<t>& a)
{
if (this == &a)
{
FatalErrorIn("Matrix<t>::operator=(const Matrix<t>&)")
<< "attempted assignment to self"
<< abort(FatalError);
}

T* v = v_[0]; // before deference, need to allocate memory for v_ and deallocate the memory for v_ allocated avoiding memory leak.
const T* av = a.v_[0];

label nm = n_*m_;
for (register label i=0; i<nm; i++)
{
v[i] = av[i];
}
}



test:
Matrix<scalar> a(2,2,3);
Matrix<scalar> b(2,2,1);

Matrix<scalar> c=b; //OK copy constructor

Matrix<scalar> d;
d=a+b; // error, segmentation fault! the memory of d is not allocated.

Info<<a+b<<endl; //OK copy constructor

Is it true ???

Junwei

niklas October 16, 2007 01:56

The problem is yours I'd say.
 
The problem is yours I'd say.
Matrix<scalar> d;

where is the size set in this?

Im a bit surprised that it's allowed to have a null constructor actually.

It's like trying to add vectors a and b
vector a = (1)
vector b= (1,2)

henry October 16, 2007 03:11

Currently the assignment opera
 
Currently the assignment operator assumes that the LHS is the same size as the RHS and copies the elements which clearly in your case does not work. I think that the code should check that the sizes of the LHS and RHS are the same. For your code to work you would need to pre-allocate the matrix d with the appropriate sizes.

Given that null-construction is allowed perhaps it would be better if the LHS is cleared and resized automatically to match the RHS (as is the case to List for example) in which case your code would then work as-is. I will make this change and post the code.

su_junwei October 16, 2007 03:27

Hi Niklas and Henry actu
 
Hi Niklas and Henry

actually, the size of the RHS can be assigned in the definition of the function as

template<class>
void Matrix<t>::operator=(const Matrix<t>& a)
{

if (this == &a)
{
FatalErrorIn("Matrix<t>::operator=(const Matrix<t>&)")
<< "attempted assignment to self"
<< abort(FatalError);
}
if(a.empty()) //check if a is empty(),
{
FatalErrorIn("Matrix<t>::operator=(const TMatrix<t>& a)")
<< "a is empty: a.n_= "<<a.n_<<" a.m_="<<a.m_<<nl
<< abort(FatalError);
}

if(a.n_!=n_||a.m_!=m_)
{
deallocate(); //a function deallocate the memory allocated for LHS
n_=a.n_;
m_=a.m_;
allocate(); //reallocate
}

T* v = v_[0];
const T* av = a.v_[0];
label nm = n_*m_;
for (register label i=0; i<nm; i++)
{
v[i] = av[i];
}
}

the code is similar to the definition of assignment operator of List at

/src/OpenFOAM/containers/Lists/List/List.C

Thus,It seems that all work done.

yours

Junwei


All times are GMT -4. The time now is 22:28.