deallocating gsl vectors in structs

Question

Can you explain what's going on with my code here? I'm not sure if I'm using the destructor correctly or not in the struct.

With the destructor in there I get:
function1: 23
function2: 8.86183e-317
* glibc detected ./a: double free or corruption (fasttop): 0x000000000111b010 **

If I just comment out the destructor I get:
function1: 23
function2: 24

which is what I want. But don't I need the destructor to avoid a memory leak for a more complicated program?

(As you can see I may be a bit confused on pointers/allocation in general)

Thanks!

Edit: oh yeah, and why does the extra allocation step in function1 make a difference?

Edit2: Should I be initializing x = 0 in the constructor? I thought that was proper...should I be allocating it on initialization when I do this? So instead: x = gsl_vector_alloc(1).

#include <iostream>
    using namespace std;
#include <cassert>
#include <cmath>
#include <gsl/gsl_vector.h>

struct struct1{
    gsl_vector * x;

    struct1() {
        x = 0;
    }
    ~struct1() {
        if (x) gsl_vector_free(x);
    }
};

void function1(void *p) {
    struct1 s = *(struct1 *) p;
    s.x = gsl_vector_alloc(1);
    gsl_vector_set(s.x, 0, 24);
}

void function2(void *p) {
    struct1 s = *(struct1 *) p;
    gsl_vector_set(s.x, 0, 24);
}

int main() {
    struct1 s;
    s.x = gsl_vector_alloc(1);
    gsl_vector_set(s.x, 0, 23);

    function1(&s);
    cout << "function1: " << gsl_vector_get(s.x, 0) << endl;

    function2(&s);
    cout << "function2: " << gsl_vector_get(s.x, 0) << endl;

    return 0;
}

score 1 · Accepted Answer · answered Dec 03 '10 at 18:13

1

Inside of function1 and function2 you make a copy of the struct1 object that you create in the main() function. These copies have the same pointer x. When the destructor for each of these copies is called, gsl_vector_free is called, so you try to call it three times on the same pointer:

once in function1 when s is destroyed
once in function2 when s is destroyed
once in main when s is destroyed

You need to implement a copy constructor and copy assignment operator for this class. Any time you have a class that owns a resource, you need to implement these two functions and a destructor. A resource is anything that needs to be cleaned up when you are done using it.

In your sample code, it would be far better to encapsulate all of the allocation and deallocation inside of the class so that you can use the class without worrying about it. Make the class actually manage its resource.

answered Dec 03 '10 at 18:13

James McNellis

348,265
75
913
977

Ok, but what if I don't know how big the vector x should be when I initialize the struct (class)? Is it a bad idea to allocate it to size 1 and then later reallocate it when I know what size it should be? – evencoil Dec 03 '10 at 18:20
@evencoil: Can you provide a `resize()` member function? – James McNellis Dec 03 '10 at 18:22
I'm not sure what the reasons would be that I couldn't...I think I know how to do that, I just didn't know I should. What's the difference between doing say alloc(1), free, alloc(5) as opposed to alloc(1), alloc(5) ? In the past I haven't found this to be problematic for gsl_vectors....but I don't have that much experience. – evencoil Dec 03 '10 at 18:26
@evencoil: I don't know; you'd have to consult the library's documentation. On another note, do you have [a good introductory C++ book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list)? – James McNellis Dec 03 '10 at 18:28
I see, because presumably gsl_vector_alloc might check to see whether the vector is allocated first, if it is, then deallocate it, and then reallocate to the new size? I will check the code/documentation and see if its doing that. I do have some C++ references, but I'm a scientist not a programmer so I'm limited in how much time I can commit to really learning c++ all the way through before using it. Sorry if the question was too basic. – evencoil Dec 03 '10 at 18:32
@evencoil: No, no question is too basic. It would just be a good idea to have a good introductory book if you are going to be working with C++. It's not an easy language to learn and there are many, many common pitfalls. – James McNellis Dec 03 '10 at 18:33
Ok. Well thanks for the help I learned a bit more. Also, in response to the void * question its because that's what gsl wants when you supply it a function to (for example) differentiate--the void pointer carries all of the parameters and then its up to you to make sure you cast it correctly. – evencoil Dec 03 '10 at 18:38
To clarify: it wants function(double d, void * p) and then differentiates wrt to d. – evencoil Dec 03 '10 at 18:39
Sorry, I'm confused by that--I'm calling them in main()? I think the crash occurs when the program ends but what I was more worried about is that function2 affects s.x in main differently depending on whether there's a destructor or not. – evencoil Dec 03 '10 at 20:08
Thanks you've helped me a lot here. The key was I didn't understand the copy constructor and what saying "a = b" for a class means. All I was doing was equating the gsl_vector *, but not the memory associated with it. The easiest solution for my application is doing: struct1 * ps = (struct1 *) p; and then just using ps->x later on. And I realize this just makes my code cleaner--I could also do ((struct1 *) p)->x everywhere and it would have the same effect. – evencoil Dec 03 '10 at 21:18

deallocating gsl vectors in structs

1 Answers1