Jump to content
• Advertisement

# Memory leaks

This topic is 4882 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

## Recommended Posts

Hi, i wrote some math and physiks library to implement some realistic opengl graphics. At the moment it works fine with some balls(velocity force torque ......). But if i turn on destructor in my Vector class, i get segmentation faults. But if i turn it of, i get memory leaks, so big, that after 5 minutes the programm takes 50 % of the memory ..... so myby somebody can help me, here is some code example: class Vektor { public: Vektor(int); Vektor(int, int); ~Vektor(); Vektor operator = (Vektor); Vektor operator + (Vektor); Vektor operator * (Vektor); Vektor operator - (Vektor); Vektor operator * (double); static double euclideanScalarProduct(Vektor*, Vektor*); double euklidnorm(); double unorm(); void setvalue(int, double); void setvalue(double*); double* getvalue(); double getvalue(int); int getdim(); double* a; int dimension; }; #include "Vektor.h" #include <stdio.h> #include <math.h> Vektor::Vektor(int dim) { a=new double [dim]; dimension = dim; for (int i =0;i<dim;i++) { a=0; } } Vektor::Vektor(int dim, int k) { a=new double [dim]; for (int i =0;i<dim;i++) { a=0; } a[k]=1; dimension=dim; } Vektor::~Vektor() { if(a != NULL) { //delete[] a; //if i turn on, i get segfault... } } Vektor Vektor::operator = (Vektor v) { dimension=v.dimension; if(a == NULL) { a = new double[dimension]; } for (int i=0; i<v.getdim();i++) { a=v.a; } return(*this); } Vektor Vektor::operator + (Vektor v) { Vektor erg(v.dimension); erg.dimension=dimension; for (int i=0; i<v.dimension;i++) { erg.a=a+v.a; } return(erg); } Vektor Vektor::operator * (Vektor v) { if(dimension != 3) { return (*this); } else { Vektor erg(dimension); erg.dimension=dimension; erg.a[0] = a[1]*v.a[2] - a[2]*v.a[1]; erg.a[1] = a[2]*v.a[0] - a[0]*v.a[2]; erg.a[2] = a[0]*v.a[1] - a[1]*v.a[0]; return(erg); } } Vektor Vektor::operator - (Vektor v) { Vektor erg(dimension); erg.dimension=dimension; for (int i=0; i<dimension;i++) { erg.a=a-v.a; } return(erg); } Vektor Vektor::operator *(double s) { Vektor erg(dimension); erg.dimension=dimension; for (int i=0; i<dimension;i++) { erg.a=s*a; } return(erg); } double Vektor::euklidnorm() { double sp=0; for (int i=0; i<dimension ;i++) { sp=sp+a*a; } sp = sqrt(sp); return(sp); } double Vektor::unorm() { double erg=0; for (int i=0; i<dimension ;i++) { if (fabs(erg)<=fabs(a)) { erg=fabs(a); } else {} } return (erg); } void Vektor::setvalue(int pl, double we) { a[pl]=we; } void Vektor::setvalue(double* v) { a=v; } double* Vektor::getvalue () { return(a); } double Vektor::getvalue(int pl) { return(a[pl]); } int Vektor::getdim() { return(dimension); } double Vektor::euclideanScalarProduct(Vektor *v, Vektor *w) { double scalar = 0; for (int i=0; i<v->dimension ;i++) { scalar += v->a * w->a; } return scalar; } thnx

#### Share this post

##### Share on other sites
Advertisement

It is VERY bad to deallocate memory in the destructor with an object like a vector. Why are you using allocated memory in the first place? I'd recommend using stack memory and just creating vec2, vec3, and vec4 classes. If you really need more flexibility, you can allocate memory, but you have to be really careful.

Why? Because many people pass objects like vectors by value. You should try to pass it by reference whenever possible, but you can't guarantee that. So, what happens when you pass it by value? Well, I felt like drawing just now for some reason, so...

Basically you are calling delete[] twice on the same block of memory because when you pass by value, it creates a copy of the object which inappropriately nukes the memory.

Hope that's clear, I'm in a weird mood. :p

#### Share this post

##### Share on other sites
you need to implement Vektor(const Vektor&) i.e. the copy ctor to have pass by value work correctly else you'll wind up double deleting, that's (probably)what's causing the segfault.
something like:
Vektor::Vektor(const Vektor &other):{  a = new double[other.dimension];  dimension = other.dimension;  for(int i = 0; i != dimension; ++i)    a = other.a;}

also you might want to get into the habit of passing by const reference instead of (like you do now) passing by value since you'll get rid of a lot of copying overhead that way.

#### Share this post

##### Share on other sites
Your operator=(Vektor v) will cause a buffer overflow if v.getdim() is bigger than this->getdim().

#### Share this post

##### Share on other sites
Following DD's post, you need to make the other operators use const Vektor references too, or just Vektor references for those that modify the parameter.

You really REALLY should not be using dynamic memory for a vector, that's massive overkill.

Try using a well written class like this

#### Share this post

##### Share on other sites
Quote:
 Original post by DigitalDelusionyou need to implement Vektor(const Vektor&) i.e. the copy ctor to have pass by value work correctly else you'll wind up double deleting, that's (probably)what's causing the segfault. something like:Vektor::Vektor(const Vektor &other):{ a = new double[other.dimension]; dimension = other.dimension; for(int i = 0; i != dimension; ++i) a = other.a;}also you might want to get into the habit of passing by const reference instead of (like you do now) passing by value since you'll get rid of a lot of copying overhead that way.

And having made this work correctly, consider implementing the assignment operator in terms of the copy ctor, by the copy-and-swap idiom:

Vektor& Vektor::operator=(const Vektor& rhs) {  Vektor other(rhs); // if this throws, the object gets destroyed and   // releases its memory, and the rest is skipped by the exception.  std::swap(a, other.a); // swap pointers; now we own the "new" array, and  // "other" owns our old data  dimension = other.dimension;  // At the end of the method, the "other" Vektor falls out of scope,  // triggering its dtor, and thus cleaning up its array - which is the old  // array from the current object, due to the swap.}

At which point it becomes possible (and really the only correct option) to use the old dtor, "delete[] a". BTW, the null checks are not necessary with C++ operators delete and delete[]; calling them on a null pointer is a no-op. (Calling them on an *invalid, non-null* pointer is undefined, but there's no portable way to detect an invalid pointer *anyway*.)

#### Share this post

##### Share on other sites
ok thanx a lot folks. Helped me a lot :-)

#### Share this post

##### Share on other sites

• Advertisement
• Advertisement

• ### Popular Contributors

1. 1
Rutin
34
2. 2
3. 3
4. 4
5. 5
• Advertisement

• 12
• 14
• 9
• 9
• 9
• ### Forum Statistics

• Total Topics
633336
• Total Posts
3011412
• ### Who's Online (See full list)

There are no registered users currently online

×

## Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!