Crashing while deleting

Question

0.00/5 (No votes)

See more:

Hello
I am running this code in VS2008.
I have changed some project settings:
Character Set:Use multi byte character set
Structure member alignment: 1 Byte
After changing these settings, this code crashes while deleting last entry.
If I dont change the settings, then it works fine.

struct licenseClient
{
	SOCKET  socketNo;
};

struct licenseClientData
{
	int				iCount;
	licenseClient	**clientData;
};

int _tmain(int argc, _TCHAR* argv[])
{
	licenseClientData temp;
	temp.clientData = new licenseClient*;
	temp.iCount = 0;

	temp.clientData[temp.iCount] = new licenseClient;
	temp.iCount++;
	temp.clientData[temp.iCount] = new licenseClient;
	temp.iCount++;
	temp.clientData[temp.iCount] = new licenseClient;
	temp.iCount++;

	temp.iCount--;
	delete temp.clientData[temp.iCount];
	temp.iCount--;
	delete temp.clientData[temp.iCount];
	temp.iCount--;
	delete temp.clientData[temp.iCount];

	getch();
	return 0;
}

I am unable to understnd why this is crashing?

thanks
Nipun

[edit]Code block added, "Ignore HTML..." option disabled - OriginalGriff[/edit]

Posted 13-Jul-11 22:40pm

NipunG

Updated 13-Jul-11 22:43pm

OriginalGriff

v2

Add a Solution

3 solutions

Solution 2

Cpallini answered part of the question; but I am curious as to why you wrote the code this way. Are you just learning C++? Just experimenting with 'new' and 'delete'? Or is there some other reason?

#include <vector>
// Your way
void do_something_1()
{
    licenseClientData temp;
    temp.clientData = new licenseClient*[3]; // pointer to array of pointers to  licenseClient
    temp.iCount = 0;

    // ….

    temp.iCount--;
    delete temp.clientData[temp.iCount];
    temp.iCount--;
    delete temp.clientData[temp.iCount];
    temp.iCount--;
    delete temp.clientData[temp.iCount];

    delete []  temp.clientData; // remember to delete the array as well
    getch();
    return 0;
}
// Better way
void do_something_2()
{
    std::vector<SOCKET> licenseClient(3,(SOCKET()));
    // do something with array
}
// Another way
void do_something_3()
{
    std::vector<SOCKET> licenseClient;
    licenseClient.push_back((SOCKET()));
    licenseClient.push_back((SOCKET()));
    licenseClient.push_back((SOCKET()));
    // do something with array
}
// Or even
void do_something_4()
{
     SOCKET licenseClient[3] = {SOCKET(), SOCKET(), SOCKET()};
    // do something with array
}

Note that use of 'new' and 'delete' is rarely required – unless you are going to pass the pointer around or you need the object to have a longer life-time.

Sorry about all the edits - but to editor keeps messing up my code.

Posted 14-Jul-11 14:43pm

John R. Shaw

Updated 14-Jul-11 23:18pm

v6

Comments

John R. Shaw 14-Jul-11 20:53pm

I give - it insist on adding </vector> to the end - I the rest looks fixed.

Stefan_Lang 15-Jul-11 5:20am

fixed your </vector> problem by replacing the angular brackets of your include statement

John R. Shaw 15-Jul-11 9:53am

Doh! Thanks :D

Solution 3

Use STL instead.

C++

#include <vector>

struct licenseClient
{
    SOCKET  socketNo;
};

struct licenseClientData
{
    // Not required anymore uses clientData.size() instead.
    // int             iCount;

    std::vector<licenseClient > clientData;
};

int _tmain(int argc, _TCHAR* argv[])
{
    licenseClientData temp;

    licenseClient lc1;
    temp.clientData.push_back(lc1);

    licenseClient lc2;
    temp.clientData.push_back(lc2);

    licenseClient lc3;
    temp.clientData.push_back(lc3);

    // The following is not necessary if the data 
    // is not used elsewhere in the real program.
    temp.clientData.erase(temp.clientData.back());
    temp.clientData.erase(temp.clientData.back());
    temp.clientData.erase(temp.clientData.back());

    getch();
    return 0;
}

In real life, you would probably add a check to ensure that you still have item to remove.

C++

if (temp.clientData.size() > 0)
{
    temp.clientData.erase(temp.clientData.back());
}

Posted 14-Jul-11 14:46pm

Philippe Mori

Updated 15-Jul-11 3:53am

v3

Comments

Philippe Mori 14-Jul-11 20:57pm

The are possible variations around this idea like using auto-ptr or similar classes if you really need to manage dynamically allocated objects.

ThatsAlok 15-Jul-11 8:10am

what about temp.clientData.clear()

Philippe Mori 15-Jul-11 9:51am

Effectively, if all items can be removed at the same time, clear is a better idea. I simply done it that way so that it would be somehow similar to the original code.

Philippe Mori 15-Jul-11 9:56am

I just made a correction in the solution. I forgot to remove iCount.

By the way when using STL (or templates in general), variable name should not be decorated with prefix for their types...

Add a Solution

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

CPallini · Accepted Answer · 2011-07-13T23:07:00

Solution 1

you are using temp.clientData as an array but you didn't allocate memory for it.
Namely, in the posted code you should change

C++

temp.clientData = new licenseClient*;

to

C++

temp.clientData = new licenseClient*[3];

Posted 13-Jul-11 23:07pm

CPallini

Comments

Albert Holguin 14-Jul-11 9:53am

that's probably the problem, my 5

CPallini 14-Jul-11 14:31pm

Thnak you.

Harrison H 14-Jul-11 14:02pm

My 5 too. I'd also ask the guy why he doesn't use the damn facilities given to him by C++. He wouldn't have messed this up with better containers, and I bet he doesn't really need the performance "boost" of raw arrays.

CPallini 14-Jul-11 14:31pm

Thanks.
Such 'performance boost' is rarely needed. I think most of the folks are simply scared by STL containers.

ThatsAlok 15-Jul-11 8:29am

nice solutions deserve 5ed+

Crashing while deleting

3 solutions

Solution 1

Solution 2

Solution 3

Add your solution here

Preview 0

Crashing while deleting

3 solutions

Solution 1

Solution 2

Solution 3

Add your solution here

Preview 0

Existing Members

...or Join us