problem while freeing a link list within a vector structure [modified]

[Madan Chauhan]

Hi all.
I have a link list in a vector structure. It throws exception while freeing first node of the list. This is the sample code-

//////////Declaration//////
typedef struct _TEST_STRUCT
{
TCHAR _tchTemp[MAX_PATH];
__int64 i64Temp;
DWORD dwTemp;
_TEST_STRUCT *Next;
}TEST_STRUCT;

typedef struct _TEST_VECTORST
{
int nVal;
TEST_STRUCT m_ListInVector;

}TEST_VECTORST;

std::vector<TEST_VECTORST> m_StVector;
////////////////////////////////////////////////////
/////////Fill Structure//////////////

CString csVal = L"";
csVal         = L"hello";
for (int i = 0; i < 2; i++)
{
	memset (&wVectorSt, 0x00, sizeof(TEST\_VECTORST));
	ptrList = NULL;
	wVectorSt.nVal = i + 1;
	int j = 0;
	while (j<10)
	{
		if (ptrList == NULL)
		{
			ptrTemp = (TEST\_STRUCT\*)calloc(1,sizeof(TEST\_STRUCT));
			\_tcscpy(ptrTemp->\_tchTemp, csVal);
			ptrTemp->dwTemp = 10+j;
			ptrTemp->i64Temp = 100+j;
			ptrTemp->Next = NULL;
			ptrCurrent = ptrList = ptrTemp;
		}
		else
		{
			ptrTemp = (TEST\_STRUCT\*)calloc(1,sizeof(TEST\_STRUCT));
			\_tcscpy(ptrTemp->\_tchTemp, csVal);
			ptrTemp->dwTemp = 10+j;
			ptrTemp->i64Temp = 100+j;
			ptrTemp->Next = NULL;
			ptrCurrent->Next = ptrTemp;
			ptrCurrent       = ptrTemp;

		}
		j ++;
	}
	memcpy(&wVectorSt.m\_ListInVector, ptrList, sizeof(TEST\_STRUCT));
	m\_StVector.push\_back (wVectorSt);
}
//////Free m\_StVector.m\_ListInVector////
for (int i = 0; iNext;
		free(ptrTemp);
		ptrTemp = NULL;
	}
}  

All the nodes become free but one that is very first node in the list does not free.
if i take start to free from first node like- ptrList = &m_StVector[i].m_ListInVector;

then it throws exception immidiately and does not free any node...

Please give me some idea...

Thanks

modified on Tuesday, June 21, 2011 4:17 AM

[CPallini]

Please use the 'code block' button to format properly your code.

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[Stefan_Lang]

You did not dynamically allocate the first member, so you cannot dynamically free it either. More to the point: when you push_back() a struct object into that vector of yours, the vector will allocate the required memory and copy the struct you passed into that memory location. The vector is thus responsible for the releaese of that storage, not you. Since you didn't allocate the vector on the heap, it will be automatically deallocated when the variable leaves the scope. But if you defined it globally, then you might want to enforce this earlier, by calling

m_StVector.clear()

.

[Madan Chauhan]

thanks for the response. Yes i have declared m_StVector in App class. So i need to clear it. but even i do m_StVector.clear() then still i see those memory leaks that are due to the first node of the link list... well I am still stuck how to free those.

[CPallini]

Why are you using your own list, cannot you use the STL one? Why are you using C allocation functions? What is wVectorSt?

If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler. -- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong. -- Iain Clarke
[My articles]

[Stefan_Lang]

I just noticed that of course you need to free the strings within your struct, and that is something your vector will not take care of. The best way would be to define both a constructor and destructor for your struct, and use new/delete instead of calloc/free. In the destructor you can then just take care of the string:

struct TestStruct {
TestStruct() : tchTemp(0) {} // constructor initializes string pointer to 0
~TestStruct() { delete [] tchTemp; } // destructor frees string if needed
void set(const TCHAR* stringvalue) {
delete [] tchTemp; // release previously allocated memory
if (stringvalue != 0) {
// create a copy
tchTemp = new TCHAR[_tcslen(stringvalue)+1];
_tcscpy(stringvalue, tchTemp);
}
else {
// reset pointer to 0
tchTemp = 0;
}
}
TCHAR* tchTemp;
};

void test() {
std::vector struct_vect;
// now fill vector and work with it
// ...
struct_vect.clear(); // will delete and deconstruct all objects
}

Note that in order to actually deconstruct (i. e. call the constructor of) the TestStruct objects, you must call delete, not free, on any dynamically allocated objects, and thus you must call new to allocate it. Also you must use new to allocate your strings if you want to use delete to free them. I added a set function to simplify the process of assigning this string properly, using new. Of course, if you don't want to change your struct definition, you can also loop over the element vectory and free() tchTemp from your first nodes manually.

[David Crow]

Madan Chauhan wrote:

All the nodes become free but one that is very first node in the list does not free.

How are you verifying this?

Madan Chauhan wrote:

Please give me some idea...

Just a tad bit of cleanup:

void main( void )
{
for (int i = 0; i < 2; i++)
{
TEST_STRUCT *ptrCurrent;
TEST_VECTORST wVectorSt = {0};
wVectorSt.nVal = i + 1;

    for (int j = 0; j < 10; j++)
    {
        TEST\_STRUCT \*ptrTemp = (TEST\_STRUCT \*) calloc(1, sizeof(TEST\_STRUCT));
        \_tcscpy\_s(ptrTemp->\_tchTemp, \_countof(ptrTemp->\_tchTemp), \_T(""));
        ptrTemp->dwTemp = 10 + j;
        ptrTemp->i64Temp = 100 + j;
        ptrTemp->Next = NULL;

        if (wVectorSt.m\_ListInVector == NULL)
            wVectorSt.m\_ListInVector = ptrTemp;
        else
            ptrCurrent->Next = ptrTemp;

        ptrCurrent = ptrTemp;
    }

    m\_StVector.push\_back(wVectorSt);
}

for (unsigned int i = 0; i < m\_StVector.size(); i++)
{
    TEST\_STRUCT \*ptrList = m\_StVector\[i\].m\_ListInVector;
    while (ptrList != NULL)
    {
        TEST\_STRUCT \*ptrTemp = ptrList;
        ptrList = ptrList->Next;
        free(ptrTemp);
    }
} 

}

"One man's wage rise is another man's price increase." - Harold Wilson

"Fireproof doesn't mean the fire will never come. It means when the fire comes that you will be able to withstand it." - Michael Simmons

"Some people are making such thorough preparation for rainy days that they aren't enjoying today's sunshine." - William Feather