|
progDes wrote: Destructor must call it.
But it doesn't and memory leak is shown on my debug window. When I explicitly call Empty however it disappears.
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
I already know what the problem is from the first time I saw a ‘new’ statement. I finished a course on modern programming languages (MS Specific), so I assume you think it is magic. Wrong, it is only magic if the language supports automatic clean up! The ‘new’ operator allocates memory and it is up to you to free it; otherwise you will have a memory leak. In Microsoft’s new languages it is still a good idea to free them, because they go into a list somewhere, waiting to be released.
This is relates to a very old problem that I never understood. If you allocate the memory, then you need to free it. What is the problem?
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|
|
John R. Shaw wrote: The ‘new’ operator allocates memory and it is up to you to free it;
What's the point here? I already delete them, but didn't add delete in this code, but wrote it when I said:
The following code, has memory leak. (Not in m_pA and pStr, since I delete them later )
I understand how to allocate and deallocate memory either in C and C++. My problem here is that:
when I assign a pointer-to-a-char*-buffer to a CString object, I then have to call Empty() of that CString object. This is what I don't understand. I never read anywhere that CString requires programmer to call it's Empty function, and if I don't I'll have memory leaks.
I modified my code and added those deletes that I talked about. Now I think it's even more clear that I delete those newed variables.
-- modified at 10:17 Saturday 8th September, 2007
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
|
Thanks.
I'll soon read it.
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
From MSDN:
In previous versions of MFC, it was important that you clean up CString objects after use. With MFC version 3.0 and later, explicit cleanup is no longer necessary.
May be you have MFC 3.0?
|
|
|
|
|
progDes wrote: May be you have MFC 3.0?
I'm not sure. I use VS 6.0 SP5.
This might seems stupid, but I don't know how can I find my MFC version?
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
I'm thinking maybe you are trashing the heap with this:
m_pA->pStr = new char[260];
CString szPath = m_pA->pStr; You haven't initialized the array that's being copied to the CString.
What if you do this...
m_pA->pStr = new char[260];
m_pA->pStr[0] = 0;
CString szPath = m_pA->pStr; I'm not sure what you're trying to accomplish here, however.
Mark
Mark Salsbery
Microsoft MVP - Visual C++
|
|
|
|
|
Thanks for your reply Mark. I have recently many question answered by you. You deserve being a CP MVP, if you are not already.
Mark Salsbery wrote: You haven't initialized the array that's being copied to the CString.
Actually I did. I hope you are patient enough to read this similar to real code:
For easier reading I ignored .h and .cpp and wrote them at once.
Class a
{
void SetPath(CString strPath)
{
memset(m_pA->pSzPath, NULL, MAX_PATH);
int len = min(MAX_PATH, strPath.GetLength());
strncpy(m_pA->pSzPath, strPath, len);
m_pA->bPathUpdated = TRUE;
}
static UINT ThumbnailViewerThread(LPVOID pParam)
{
CString strFullPath, strFileName, strFolderPath;
while(!pData->bStop)
{
if (pClientData->bPathUpdated)
{
strFolderPath = pClientData->pSzPath;
strFullPath = strFolderPath + "\\" + strFileName;
strFolderPath.Empty();
}
else
{
sleep(500);
}
}
}
}
I call SetPath member function to set a new path for the thumbnail thread reader.
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
Hamed Mosavi wrote: void SetPath(CString strPath)
{
memset(m_pA->pSzPath, NULL, MAX_PATH);
int len = min(MAX_PATH, strPath.GetLength());
strncpy(m_pA->pSzPath, strPath, len); // Path to folder
m_pA->bPathUpdated = TRUE; // a volatile BOOL
}
There still may be a problem with the string termination, possibly causing memory corruption or overrun.
Assuming the size of the pSzPath is MAX_PATH, maybe try:
void SetPath(CString strPath)
{
memset(m_pA->pSzPath, 0, MAX_PATH);
int len = min(MAX_PATH<font color="Red"> - 1</font>, strPath.GetLength());
strncpy(m_pA->pSzPath, strPath, len);
<font color="Red"><font><font> m_pA->pSzPath</font></font>[len] = 0;</font>
<font> m_pA->bPathUpdated = TRUE;
}</font> You shouldn't need the empty call unless you want to explicitly empty the string.
Mark
Mark Salsbery
Microsoft MVP - Visual C++
|
|
|
|
|
Mark Salsbery wrote: (MAX_PATH - 1,
That's right. I took a closer look at my code and I saw that -1 in mine. Sorry to forget to add it here.
Why do I need to call this:
m_pA->pSzPath[len] = 0;
when I just called this:
memset(m_pA->pSzPath, 0, MAX_PATH);
len will be MAX_PATH-1 or something smaller, so that this char* always has the \0 in the end, before being filled with strncpy . This \0 is not the problem here. I think I might need to find some free time and check the resulting assembly code, perhaps I find something there.
Mark Salsbery wrote: You shouldn't need the empty call
That's what bothers me here. There's obviously something's wrong
Anyway thank you so much for your kind help. I appreciate it.
Thanks Mark.
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
You're right - never mind about the NULL termination then
How are you detecting the memory leak?
The CString destructor should free the contents of the string. If this isn't happening
then the destructor isn't getting called.
Is your thread shutdown all the way before the app exits?
Mark
Mark Salsbery
Microsoft MVP - Visual C++
|
|
|
|
|
Mark Salsbery wrote: the destructor isn't getting called.
Is your thread shutdown all the way before the app exits?
Nope! Both destructor and thread work just fine! After I finish the job, I'll be back to find the reason even if I have to use a disassembler and read assembly code, because it tried me crazy and got a lot of my time, and of course yours.
At the moment, if you are interested, I took a snapshot of the thread and the debug window and uploaded them here:
I removed all codes from the thread to make it simpler. even this code has the memory leak.
http://www.sarvsoft.com/CrazyMemLeak/1.jpg
http://www.sarvsoft.com/CrazyMemLeak/2.jpg
If you are interested in GUIs, here's an image of the GUI but in my own language. Inspired by Vista. I'm currently implementing that left most innocence(!) thumb list:
http://www.sarvsoft.com/CrazyMemLeak/3.jpg
// "Life is very short and is very fragile also." Yanni while (I'm_alive) { cout<<"I love programming."; }
|
|
|
|
|
My application was generared with wizard. It'is SDI.
The problem:
The document do not automatically attach to the view.
The document is creating but the m_pDocument in the view class is always NULL.
How can I solve this problem?
-- modified at 2:03 Saturday 8th September, 2007
|
|
|
|
|
thats vary simple.
say for example,
your sdi application is named as example,
then your view will be CexampleView
and your document class will be CexampleDoc.
now your requirement is to get the object of CexampleDoc in CexampleView right?,
if it is so,
the following is the logic.
in CexampleView class(.h file) define a variable like this.
CexampleDoc *pDoc;
and in constructor or ondraw,
pDoc=GetDocument();
now you can access the document class with pDoc.
|
|
|
|
|
Thanks but I know document/view architecture. I found the problem.
IMPLEMENT_DYNCREATE(CSalesmanView, CScrollView)
BEGIN_MESSAGE_MAP(CSalesmanView, CScrollView)
changed to
IMPLEMENT_DYNCREATE(CSalesmanView, CFormView)
BEGIN_MESSAGE_MAP(CSalesmanView, CFormView)
I was changed the parent of CSalesmanView from CScrollView to CFormView by hand and forgot to change this.
SOLVED
|
|
|
|
|
I am working on a treeview (straight win32) and was wondering if anyone knew how I could change the text apperance? (Yes, I know how to change text color in general). I can not seem to find an obvious way to do this. I have tried using TreeView_SetTextColor but that will change the text color for all of tree. I only want to change one branch of my tree.
http://rafb.net/p/7VhWKn68.html (tree.h) and http://rafb.net/p/6ltyVn44.html (tree.cpp)
|
|
|
|
|
Handle NM_CUSTOMDRAW , you can change the text and background colors for each item.
|
|
|
|
|
I have looked up using NM_CUSTOMDRAW but I am not sure how I would use it with my code. How would I use it in relation to my code?
|
|
|
|
|
See this article[^], it's about list views but the procedure is similar for tree views.
|
|
|
|
|
Good article, however, I noticed they used there list as a class but I do not.
Also, could I use this function to change the font size?
|
|
|
|
|
Hello all,
I'm having a problem when trying to free a string that is pointed from a member of colName array.
That is of course after I'm allocation memory for the colName array, and each of it's members.
the struct i'm working on:
typedef struct
{
char** colName;
const struct element_interface** ifc;
Element* temp_row;
}data;
typedef struct table_rec
{
char TabInitFlag;
int iterator;
data* localData;
char** last_found_array;
Matrix tableArray;
int tableWidth;
int last_found;
}table_rec;
The command that I'm using to free the string.
int num;
for(i = 0; i < num; i++)
{
free(colNum[i]);
}
The error I'm receiving in the first iteration (as any):
Unhandled exception at 0x00418780 in project.exe: 0xC0000005: Access violation reading location 0xfdfdfe01.
Thank you,
Gizmokaka
|
|
|
|
|
gizmokaka wrote: That is of course after I'm allocation memory for the colName array, and each of it's members.
Just to confirm, you are allocating memory for the pointer that points to an array of pointers, correct? Something like:
char **myArray = malloc(sizeof(char *) * 10);
int i;
for(i = 0; i < 10; i++)
{
myArray[i] = malloc(sizeof(char) * 15);
}
|
|
|
|
|
yes that is just about how I allocated the space.
|
|
|
|
|
Wow! I am glad that was not a physical blow. (Just messing with you )
First the ‘data’ structure needs values that tells us how many items are in each array contained in the structure, otherwise we will not know how many to free.
The “data” structure contains an array of names (colName), when freeing the names you need to ensure that the pointer to the array and that each pointer to a name is valid or you will get an error (if you are lucky) or unexpected results.
To free the array do the following:
1) Check that ‘colName’ is not NULL; write an initialization function to ensure that it is NULL when it is created, similar to a C++ constructor method. You could just write “data mydata = {0}” when declaring a ‘data’ type and all members will be set to zero (NULL).
2) Before calling ‘free’ for an individual item, make sure that it is not pointing to NULL, before freeing it.
3) After calling ‘free’, set the item (colName[i]) to NULL, so you will not try to free it a second time (by accident).
4) After freeing the individual items, you need to free ‘colName’ and set it to NULL, because it not longer points to any valid data.
Example:
if( data.colName )
{
int i;
for( i=0; i < data.NameCount; ++i )
{
if( data.colName[i] )
{
free(data.colName[i]);
data.colName[i] = NULL;
}
}
free(data.colName);
data.colName = NULL;
}
If you understand all of that, then welcome to the world of real programmers!
P.S. I am ignoring the fact that your example is freeing something called colNum, which does not exist.
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|