Your cleanup(), obviously, should take as a parameter a pointer returned by fvec().
Printable View
The problem is not your DLL -- the problem is that your client/test program is misusing the DLL/API
The _MYVEC() calls a function to allocate memory, and you're calling this in a loop. Each time, you're losing the handle that was created. So where is the call to free this memory that you've allocated for each of these allocations?Code:for(int i=1; i <= _MYVEC()[0]; i++)
{
printf("%0.3f ", _MYVEC()[i]);
}
This is no different than doing this:
How are you going to call "DestroyWindow" for each window when you've lost all the window handles (except the last) that CreateWindowEx returned?Code:for (int i = 0; i < 12; ++i )
HWND hWmd = CreateWindowEx( /* whatever */ );
Or this:
How are you going to free the memory for each allocation, when you've lost the return value of "new" in each iteration of the loop (except the last one)?Code:char *p;
for (int i = 0; i < 12; ++i )
p = new char [10];
If an API or system call is documented to return a HANDLE, memory, Matrix, whatever, and it is the client's responsibility to free the handle by calling a function, then incorrect coding on the client's part doesn't mean the API or system call is faulty. There is just so much an API can do -- if the programmer using the API is coding in a faulty way, then nothing an API can do can save that programmer. The best that can be done is fully document and supply example programs that show proper usage of the functions.
As VladimirF pointed out, you should be using the pointer returned by fvec as a handle of some type, and it is the app's responsibility to use this handle when manipulating the matrix. If you want an example, look at the Windows API -- to do anything with a window, you must provide an HWND that was previously created with some sort of window creation function. Similarly, to do anything with your matrix, you supply the pointer that was returned from the Matrix creation function.
Regards,
Paul McKenzie
I think I get it now. My mistake was thinking that I could index the function pointer as one usually can do with a pointer, but that is not the case. Simply capturing the pointer returned by the function allows complete pointer indexing, and a single call to cleanup the pointer suffices. This little example works to return a vector from a dll using runtime dynamic linking without a memory leak.
Output:Code:///////////////////////////////////////////////////////////////////////////////
// mydll.h
#ifdef MY_EXPORTS
#define MY_API __declspec(dllexport)
#else
#define MY_API __declspec(dllimport)
#endif
extern MY_API double * pVector;
// Get rid of name mangeling
extern "C"
{
MY_API double * fvec(void);
MY_API void clean(double * dp);
}
////////////////////////////////////////////////////////////////////////////////////
// mydll.cpp
#include "stdafx.h"
#include "my.h"
MY_API double * pVector;
double * CreateVector(int nSize)
{
// Allocate memory including leading size containing byte
pVector = new double [nSize + 1];
memset(pVector, 0x00, nSize+1);
return pVector;
}// CreateVector(int nSize)
void FreeVector(double * pVector)
{
delete [] pVector;
pVector = NULL;
}// FreeVector(double * pVector)
MY_API double * fvec(void)
{
pVector = CreateVector(8);
pVector[0] = 8.0;
pVector[1] = 6.4342;
pVector[2] = 1.5453;
pVector[3] = 2.6546;
pVector[4] = 3.7657;
pVector[5] = 4.23423;
pVector[6] = 5.1232;
pVector[7] = 6.87654;
pVector[8] = 7.7755;
return pVector;
}
MY_API void clean(double * dp)
{
if(dp != NULL)
{
delete [] dp;
dp = NULL;
}
}
/////////////////////////////////////////////////////////////////////////////////////////
// test.cpp
#include "stdafx.h"
#include <windows.h>
#include <stdio.h>
// Function pointers that will be used for the DLL functions.
typedef double *(*FVEC)();
typedef void (*CLEAN)(double *);
int _tmain(int argc, _TCHAR* argv[])
{
int iOrg = _CrtSetDbgFlag(_CRTDBG_REPORT_FLAG);
_CrtSetDbgFlag(iOrg | _CRTDBG_LEAK_CHECK_DF);
vector<double> vf;
HINSTANCE hinstLib;
BOOL fFreeResult, fRunTimeLinkSuccess = FALSE;
FVEC _MYVEC = NULL;
CLEAN _MYCLEAN = NULL;
// Get a handle to the DLL module.
hinstLib = LoadLibrary(TEXT("my.dll"));
if (NULL != _MYVEC)
{
printf("\n_MYVEC is not NULL\n\n");
fRunTimeLinkSuccess = TRUE;
double * pVec = NULL;
// get vector
pVec = _MYVEC();
int N = (int) pVec[0];
vf.resize(N);
for(int i=1; i <= N; i++)
{
vf[i-1] = pVec[i];
}
printf("\n");
_MYCLEAN(pVec);
for(size_t i = 0; i < vf.size(); i++) { printf("%0.4f\n", vf[i]); }
} }
// Free the DLL module.
fFreeResult = FreeLibrary(hinstLib);
}
// If unable to call the DLL function, use an alternative.
if (! fRunTimeLinkSuccess)
printf("Unable to call the DLL function\n");
for(size_t i = 0; i < vf.size(); i++) { printf("%0.4f\n", vf[i]); }
return 0;
}
Handle is valid
_MYVEC is not NULL
6.4342
1.5453
2.6546
3.7657
4.2342
5.1232
6.8765
7.7755
Thank you all for your kind help and patience with a slow learner. :)
It actually can be implemented that way which would effectively mean making that vector a singleton. However, this is usually not considered good practice. And that's because if two or more parts of your program independently (i.e. without "knowing of each other") access the only one existing vector, they may have unwanted side effects on each other caused by manipulations of the commonly used vector.
Also, there's another little bug in your program:
memset() always treats the target memory block as an array of bytes, regardless of the type you actually allocated (and that memset() has no idea of). Therefore the count parameter you pass to that function always is a count of bytes. As a consequence of this, the statement above only zeroes out 1/8 of your vector. (That bug slips through unnoticed because right afterwards you fill the vector with actual doubles in fvec().) The correct instruction would look like this:Quote:
Code:memset(pVector, 0x00, nSize+1);
Code:memset(pVector, 0x00, (nSize+1) * sizeof(double));