Help with error that happens when deleting

[mcf3lmnfs]

So i have made my own dll that contains some functions. This dll has one very important class that i have defined. Now, it compiles fine, but when i try to use the functions from dll, it seems there is something horrible wrong.

Sometimes when i create some objects of that class using dynamic memory, and then when trying to delete them i receive error saying: "Unexpected error occoured" and then everything crashes.


here is code how class is defined

class matrix{
    int lime;
    int limi;
    double **table;
    public:
        //constructors
        matrix();
        matrix(int,int);
        matrix(int,int,double);
        //destructor
        ~matrix();
        //functions
        double get_element(int,int);
        void set_element(int,int,double);
        int get_width();
        int get_height();
    };
    //default constructor
    matrix::matrix(){
        }
    //primary constructor
    matrix::matrix(int a,int b){
        limi=a;
        lime=b;
        int i;
        table = new double *[a];
        for(i=0;i<a;i+=1){
            table[i]=new double [b];
            }
        }
    //secondary constructor
        matrix::matrix(int a,int b,double c){
        limi=a;
        lime=b;
        int i;
        int e;
        table = new double *[a];
        for(i=0;i<a;i+=1){
            table[i]=new double [b];
            for (e=0;e<b;e+=1){
                table[i][e]=c;
                }
            }
        }
    //destructor
    matrix::~matrix(){
        int i;
        for(i=0;i<limi;i+=1){
            delete []table[i] ;
            }
        delete []table;
        }
    ///functions
    double matrix::get_element(int a, int b){
        return table[a][b];
        }
    void matrix::set_element(int a,int b,double c){
        table[a][b]=c;
        }
    int matrix::get_width(){
        return lime;
        }
    int matrix::get_height(){
        return limi;
        }

dll functions:

double DLL_EXPORT matrix_create(double a,double b){
    //a-stevilo vrstic
    //b-stevilo stolpcev
    matrix*matrika;
    matrika=new matrix(a,b,0);
    return ((double)(int) matrika);
    }
double DLL_EXPORT matrix_destroy(double p){
    delete ((matrix*)(int)p);
    return 1;
    }
double DLL_EXPORT matrix_set_element(double p,double a,double b,double c){
    ((matrix*)(int)p)->set_element(a,b,c);
    return 1;
    }
double DLL_EXPORT matrix_get_element(double p,double a,double b){
    return ((matrix*)(int)p)->get_element(a,b);
    }
double DLL_EXPORT matrix_get_width(double p){
    return ((matrix*)(int)p)->get_width();
    }
double DLL_EXPORT matrix_get_height(double p){
    return ((matrix*)(int)p)->get_height();
    }

can anyone please try correcting either function matrix_destroy(double p) or the destructor? There has to be something wrong with one of these.

I would really appreciate your help with it.

[Tom56785]

In your 'matrix_destroy' function you are passing a regular variable rather than a pointer. You cannot deallocate memory which you have not allocated yourself, like regular variables. You would have to pass a pointer to that function instead. The parameter for that function should be matrix *p rather than a double.

Then simply use: delete p;

Similarly, you would have to pass the 'matrix' data type in your other DLL functions too. Otherwise you are casting it into a double when sent to the function, casting into an integer and then finally casting into a matrix pointer. You can probably see the issue there!


Hope this helps!

[ne555]

> ((double)(int) matrika)
¡¿?!

[andywestken]

TomTom wrote:

In your 'matrix_destroy' function you are passing a regular variable

I think the double passed to matrix_destroy() is supposed to be the "disguised" pointer value returned by matrix_create(). If that is the case, the pointer value should be recoverable and delete work OK.

This approach is pretty standard, but it is a bit unusual to see a double used for the handle type. Usually I'd expect either a void* or a typed handle. It is exactly what a lot the WinAPI functions do (e.g. CreateFile, OpenProcess, ...) A handle is used to hide he underlying type (struct or class) from the client application.

@mcf3lmnfs

I hope you realise that casting a pointer to a double is a bit unusual, and it looks rather unsafe as all your other parameters are doubles. Is there a reason you need everything to doubles? A handle type would be better here.

Actually, why are all your parameters doubles? You can't have a 2.5 x 3.75 dimension matrix, so why aren't the number of rows and columns unsigned ints? Is this an imposed choice?

Anyway...

Your destructor code looks fine, and as unusual as it is, your matrix_destroy() should work fine as long as it's passed a valid value (i.e. one returned by matrix_create() which hasn't already been destroyed.) It might be that your trying to destroy something other than a matrix you created. To trap this track what you're new-ing and deleting, e.g.

#include <vector>
#include <algorithm>
#include <cassert>

#include "testdll.h"

#include "matrix.h"

#ifdef _DEBUG
std::vector<void*> alloc_ptrs;
#endif

double DLL_EXPORT matrix_create(double a,double b){
    //a-stevilo vrstic
    //b-stevilo stolpcev
    matrix*matrika;
    matrika=new matrix(a,b,0);
#ifdef _DEBUG
    alloc_ptrs.push_back(matrika);
#endif
    return ((double)(int) matrika);
    }
double DLL_EXPORT matrix_destroy(double p){
#ifdef _DEBUG
    std::vector<void*>::iterator iter
    = std::find(alloc_ptrs.begin(), alloc_ptrs.end(), (matrix*)(int)p);
    assert(iter != alloc_ptrs.end());
    alloc_ptrs.erase(iter);
#endif
    delete ((matrix*)(int)p);
    return 1;
    }
double DLL_EXPORT matrix_set_element(double p,double a,double b,double c){
    ((matrix*)(int)p)->set_element(a,b,c);
    return 1;
    }
double DLL_EXPORT matrix_get_element(double p,double a,double b){
    return ((matrix*)(int)p)->get_element(a,b);
    }
double DLL_EXPORT matrix_get_width(double p){
    return ((matrix*)(int)p)->get_width();
    }
double DLL_EXPORT matrix_get_height(double p){
    return ((matrix*)(int)p)->get_height();
    }

The same kind of code I'm using to check that the recovered pointer corresponds to a known matrix instance before deleting it could also be added to the other API calls the check that a valid matrix is being used with them.

Andy

[ne555]

> I think the double passed to matrix_destroy() is supposed to be the "disguised" pointer value returned by matrix_create().
> If that is the case, the pointer value should be recoverable and delete work OK.
It is using int as a mediator

error: cast from pointer to smaller type 'int' loses information

> To trap this track what you're new-ing and deleting
I'd rather no touch the code, perhaps use valgrind or similar.

[mcf3lmnfs]

@andywestken, thank you a lot for the debugging idea, i will try it out as soon as possible, that is exactly what i needed, i love you.

And yes, there is a reason why i am using double. The program that is using this dll library can only receive double or char* as a return result from the dll. So each time i am passing a pointer i convert it to double and then back. The same goes for all number data types.

Thank you again for your help.

[andywestken]

And ne55 is right -- I've worked too long with Win32. But I would guess you're compiling 32-bit on Windows (I'm assuming you're working on Windows, given you're talking about DLLs) to work with some archaic app. If you ever neded to compile 64-bit then you'd have to rethink your casting r your code will work, otherwise you should be not be casting strategy.

I'd rather no touch the code, perhaps use valgrind or similar.

There are some things I find are much quicker to compile in a bit of diagnostic code than run a diagnostic tool. BoundsChecker, Purify, etc tend to slow things right down. And I am keen debug diagnostics, inc. asserts, anyway.

If I was writing this code I'd porb. be using the "handle" check code to return an appropriate return code if an invalid value was passed to the API. I would expect that's how WinAPI calls know how to return ERROR_INVALID_HANDLE rather than just die!

Andy