C / C++ / MFC

Re: Some ambigituies with MFC CString class.

7-Sep-07 19:20

2 problems:

1. The following code, has memory leak. (Not in m_pA and pStr, since I delete them later):

in .h

struct a
{
  char* pStr;
};
a* m_pA;

in .cpp

m_pA = new a;
.
.
.
// Now in a thread

m_pA->pStr = new char[260];
CString szPath = m_pA->pStr;

// Now I use szPath ( say for TRACE )
...
// And then delete the newed char* buffer
delete[] m_pA->pStr;
.
.
.
// Finally later in destructor I delete the only instance of my structure
delete m_pA;

unless I call szPath.Empty() before exiting. why?

2. Is it safe to use CString as a member of a struct and pass it to a function? with the un-predefined size of the CString how stack looks like in the function?

Thank you so much in advanced.

-- modified at 1:28 Saturday 8th September, 2007
-- modified at 10:13 Saturday 8th September, 2007
I added delete calls, since no one trusted me when at first line I mentioned I delete them later(or perhaps no one ever read the very first line;P)

// "Life is very short and is very fragile also." Yanni
while (I'm_alive)
{
cout<<"I love programming.";
}

chandu0047-Sep-07 19:26

chandu004

7-Sep-07 19:26

hi i dont suggest to use CString in a structure.
instead,
use some character array like,
char name[256];
in an array.

Re: Some ambigituies with MFC CString class.

Hamed Musavi7-Sep-07 19:36

Re: Some ambigituies with MFC CString class.

7-Sep-07 19:36

Thanks for quick reply.

chandu004 wrote:
i dont suggest to use CString in a structure.

Why?

I thought when passing a structure that has a CString to a function(as a parameter), the size of the CString is unknown, if the function changes the data, things might get worst because needed size is more than amount of already allocated memory, so an overflow. But these are just some ideas and probably wrong, because the same thing can work fine in a class.

I was wondering what happens exactly in CString class.

// "Life is very short and is very fragile also." Yanni
while (I'm_alive)
{
cout<<"I love programming.";
}

Cedric Moonen7-Sep-07 22:57

Cedric Moonen

7-Sep-07 22:57

chandu004 wrote:
i dont suggest to use CString in a structure.
instead,
use some character array like,
char name[256];

That is complete non-sense. CString class (although I prefer the stl version) is a wrapper class for a string array. Its purpose is to ease you the live so that you don't have to bother with memory allocation for your string. Why wouldn't you use it in a structure ?

Cédric Moonen
Software developer

Charting control [v1.2]

progDes7-Sep-07 20:26

7-Sep-07 20:26

2. The class is always safer then pointer. The sizeof(CString) is always identically, that's why you can easy pass it as a function parameter. Because string data is containing somewhere in heap and CString just contain pointer to this data. That's why the sizeof(CString("short string")) == sizeof("very very long string"). One more thing thing you should know, that when you pass CString as a function parameter by value the copy constructor is caling, so the data can be copied or it can be reference counting system. Anyway while the CString constructor is calling the class is safe.

1.
char * c = new char[10];
CString s = c; // the data is not pointing to c, it is copied here, so you need to delete c

Hamed Musavi7-Sep-07 20:53

7-Sep-07 20:53

progDes wrote:
2. The class is always safer then pointer.

Thanks. So it's a pointer in fact. That's really good.

progDes wrote:
// the data is not pointing to c, it is copied here, so you need to delete c

I understand it. My problem here is not c, it's s. If I don't call s.Empty() I'll have a memory leak. This really confused me.

// "Life is very short and is very fragile also." Yanni
while (I'm_alive)
{
cout<<"I love programming.";
}

progDes7-Sep-07 22:29

7-Sep-07 22:29

>"If I don't call s.Empty() I'll have a memory leak."

Destructor must call it.

Hamed Musavi8-Sep-07 4:11

8-Sep-07 4:11

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.";
}

Re: Some ambiguities with MFC CString class. [modified]

John R. Shaw7-Sep-07 22:07

John R. Shaw

7-Sep-07 22:07

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

Hamed Musavi8-Sep-07 4:08

8-Sep-07 4:08

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.";
}

bob169728-Sep-07 4:36

bob16972

8-Sep-07 4:36

Read this to understand CString's internals better to help you decide. It's slightly unrelated but it discusses parameter passing and CStrings so it might yield some ideas.

Paul DiLascia's classic walk through the CString internals[^]

Hamed Musavi8-Sep-07 4:41

8-Sep-07 4:41

Thanks.
I'll soon read it.

// "Life is very short and is very fragile also." Yanni
while (I'm_alive)
{
cout<<"I love programming.";
}

progDes8-Sep-07 6:05

8-Sep-07 6:05

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?

Hamed Musavi8-Sep-07 18:45

8-Sep-07 18:45

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.";
}

Mark Salsbery8-Sep-07 6:42

Mark Salsbery

8-Sep-07 6:42

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. Confused | :confused:

Mark

Mark Salsbery
Microsoft MVP - Visual C++
Java | [Coffee]

Hamed Musavi8-Sep-07 9:04

8-Sep-07 9:04

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);  // Path to folder
    m_pA->bPathUpdated = TRUE;  // a volatile BOOL
  }

  static UINT ThumbnailViewerThread(LPVOID pParam)
  {
    CString strFullPath, strFileName, strFolderPath;
    // blah blah
    
    while(!pData->bStop)
    {
      if (pClientData->bPathUpdated)
      {

        // Find file name by enumerating the folder

        // Replace the following 2 lines, every thing's fine! no need to Empty()

        // strFolderPath = _T("C:\\Images");
        strFolderPath = pClientData->pSzPath; // Folder path

        strFullPath = strFolderPath + "\\" + strFileName;
        // Blah blah

        // Memory leak, if I comment the following
        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.";
}

Mark Salsbery8-Sep-07 10:59

Mark Salsbery

8-Sep-07 10:59

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);  // Path to folder
<font color="Red"><font><font>    m_pA->pSzPath</font></font>[len] = 0;</font>
<font>    m_pA->bPathUpdated = TRUE;  // a volatile BOOL
  }</font>

You shouldn't need the empty call unless you want to explicitly empty the string.

Mark

Mark Salsbery
Microsoft MVP - Visual C++
Java | [Coffee]

Hamed Musavi8-Sep-07 15:53

8-Sep-07 15:53

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. Smile | :)

// "Life is very short and is very fragile also." Yanni
while (I'm_alive)
{
cout<<"I love programming.";
}

Mark Salsbery9-Sep-07 6:51

Mark Salsbery

9-Sep-07 6:51

You're right - never mind about the NULL termination then Smile | :)

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++
Java | [Coffee]

Hamed Musavi9-Sep-07 18:11

SOLVED: MFC: document-view problem [modified]

9-Sep-07 18:11

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.";
}

progDes7-Sep-07 18:41