Archived

This topic is now archived and is closed to further replies.

delete[] template error

This topic is 5774 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

man, ive been screwing with this error for liek a week. whenever i try to call the delete[], sometimes i get a critical error. *sometimes* being right after a call to delete[]. if i cant fix this error, then my entire class is useless! delete works the first time, in fStr1, but it fails the second time, in fStr2. heres my code! uncomment the destructor line to watch the error
    

#include <string.h>

#include <iostream>

#include <string>

using namespace std;


template<class myType>
class CfastString
{
	myType* _pStringStart;
	myType* _pStringEnd;

	void alignCopy(char* pCstr)
	{
		//align strlen(pCstr) to sizeof(myType)

		int length = strlen(pCstr);
		int alignedLength = (length + sizeof(myType) - length % sizeof(myType) ) / sizeof(myType);

		//set up pointers

		if( _pStringStart != NULL) delete[] _pStringStart;

		_pStringStart = new myType[alignedLength];
		_pStringEnd = _pStringStart + alignedLength;

		//set last sizeof(myType) bytes to NULL for safety

		_pStringStart[alignedLength-1] = 0;

		//copy to string

		strcpy((char*) _pStringStart, pCstr);	
	}

public:
	CfastString()
	{
		_pStringStart = NULL; _pStringEnd = NULL;
	}
	CfastString(char* pCstr)
	{
		_pStringStart = NULL; _pStringEnd = NULL;
		set(pCstr);
	}

	~CfastString()
	{
	//	delete[] _pStringStart;

	}
	
	void set(char* pCstr)
	{
		alignCopy(pCstr);
	}
	
	bool compare(CfastString fString)
	{
		myType* pString1 = _pStringStart;
		myType* pString2 = fString._pStringStart;
	
		while( pString1 != _pStringEnd)
		{
			if (*pString1 == *pString2) {
				*pString1++;
				*pString2++;
			}else
				return false;
		}
			return true;
	}

	char* pCstr()
	{
		return (char*)_pStringStart;
	}

};	//eoc CfastString


int main(int argc, char* argv[])
{
	CfastString<int> fStr1("nickkasdfa");
	CfastString<int> fStr2("nickkasdfa");

	//compare works

	if (fStr1.compare (fStr2) ) 
		cout << "same\n";
	else
		cout << "not same\n";

	//pCstr works

	cout << fStr1.pCstr() << endl;
	cout << fStr2.pCstr() << endl;

	cin.get();
	
	return 0;
}


    
Edited by - evilcrap on February 18, 2002 4:21:00 PM

Share this post


Link to post
Share on other sites
I don''t plan on reading or debugging your entire code sample, but you are doing something really quite strange that I spotted. Right at the top of your program you have:


#include <string>
#include <string.h>

You then go on to disregard any functionality in the <string> header. Why? Get rid of <string.h>, get rid of the c-string arrays, and use std::string. The class you have written is entirely redundant.

--
The Dilbert Principle: People are idiots.

Share this post


Link to post
Share on other sites
  1. Why do you include both <string> and <string.h> if you''re not going to use std::string methods?

  2. Why do you allow for an integer based string? UNICODE? I''m just asking what the rationale is behind the decision, not bashing it.

  3. Will your compare method work on strings of basic template types?

  4. I think the error lies in the allocation and copy wrangling used to transfer char * data into other types. You might want to perform your own copy operations (seek through the string until you find ''\0'', ensuring the data is properly aligned).


I wanna work for Microsoft!
[ GDNet Start Here | GDNet Search Tool | GDNet FAQ | MS RTFM [MSDN] | SGI STL Docs | Google! ]
Thanks to Kylotan for the idea!

Share this post


Link to post
Share on other sites
omfg, please dont respond if you dont know what ur talking about.

string.h is for string ops on char*.
string is so i can write a std string set function, or template.

this unoptimized class is significantly faster than std string, which was the intention.

the compare works perfectly

quote:

I think the error lies in the allocation and copy wrangling used to transfer char * data into other types.


that may be, but my tests say otherwise.



anyone: please dont respond if you dont know what your talking about, or dont bother to actually skim the source.

Share this post


Link to post
Share on other sites
quote:
Original post by EvilCrap
omfg, please dont respond if you dont know what ur talking about.


Who are you suggesting doesn't know what they are talking about?

quote:

string.h is for string ops on char*.


Yet, using char* strings is what's getting you into difficulties in the first place.

quote:

this unoptimized class is significantly faster than std string, which was the intention.


It's not faster than std::string as it doesn't even have the same functionality as std::string. You're comparing apples and oranges.

quote:

anyone: please dont respond if you dont know what your talking about, or dont bother to actually skim the source.


From where I'm sitting, it looks like you are the one in difficulty. I don't know where you're requirement for a "fast string" has come from, but you should only ever decide something is too slow when you have discovered a performance problem, profiled the application, and found that something to be the bottleneck. If you have done this, you could try investigating existing alternatives, such as vector<char>.

--
The Dilbert Principle: People are idiots.


Edited by - SabreMan on February 18, 2002 1:52:48 PM

Share this post


Link to post
Share on other sites
Gorg: did u uncomment the line in the destructor?
if u uncomment that line, the whole thing will go to hell.

SabreMan: give me a break. dont preach that trash to me, i know how to make programs. i want to write a fast string class cause i want to write a freakin fast string class. it may not have the functionality of std string cause its in development : its called unit testing, jackass.

Share this post


Link to post
Share on other sites
quote:
Original post by EvilCrap
SabreMan: give me a break. dont preach that trash to me, i know how to make programs.

And yet you''re making the very basic mistake of premature optimisation. Instead of concentrating on actually writing a program you are getting bogged down in the details of manual memory management.

quote:

i want to write a fast string class cause i want to write a freakin fast string class. it may not have the functionality of std string cause its in development : its called unit testing, jackass.

It''s your decision to waste your own time on this sort of futile pursuit, but when you go asking other people for help you would do well to tone down your arrogance and rudeness. Rather than learning through the experience of others, you are willingly committing yourself to repeat the same mistakes that have been made thousands of times before. It seems you cannot see further than your own ego.

--
The Dilbert Principle: People are idiots.

Share this post


Link to post
Share on other sites
You are right it craps out. And your problem is :


Bad copy!!


Look at compare, you are passing by value, but you do not have a copy constructor that makes a copy of the internal string, so after your call to compare, fstr2 internal strings points to crap, because the compare FastString points to same data as fstr2 data.






Edited by - Gorg on February 18, 2002 7:00:14 PM

Edited by - Gorg on February 18, 2002 7:00:47 PM

Share this post


Link to post
Share on other sites
The problem is in your compare function, if you uncomment that bit you''ll notice that everything works ok.

What happens is that you are passing fStr2 by value and so compare creates its own local copy (call it fLocal). Since you haven''t specified how it is to do this it''s anybody''s guess as to what happens, but it looks like fLocal._StringStart points to the same thing as fStr2._StringStart. delete[] is then called twice on fStr2._StringStart - once when C++ is cleaning up fLocal and once when cleaning up fStr2 which is why it segfaults.

So you can fix this by changing compare to take *CfastString as an argument. (or probably by writing your own copy constructor)

It''s good to write your own classes, but keep in mind that the standard library is actually really, really good. It was written and optimised by programmers way better than you or I and is there to be used

Share this post


Link to post
Share on other sites
quote:
Original post by Gorg
Look at compare, you are passing by value, but you do not have a copy constructor that makes a copy of the internal string, so after your call to compare, fstr2 internal strings points to crap, because the compare FastString points to same data as fstr2 data.

Yes, and this violates The Rule Of Three . The Rule Of Three is one of the most basic C++ rules for class memory management. In essence, it says that if you have to create a user-defined version of any one of the ctor, the copy ctor or the dtor, then you probably need all three.

In C++, there is not actually all that much reason to be doing this manipulation of bare pointers and basic memory blocks. Certainly, if a programmer is unaware of simple rules such as The Rule Of Three, then they are going to run into problems.

--
1st law of programming: Any given program, when running, is obsolete.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
quote:
Original post by Gorg
Look at compare, you are passing by value, but you do not have a copy constructor that makes a copy of the internal string, so after your call to compare, fstr2 internal strings points to crap, because the compare FastString points to same data as fstr2 data.
Wow, great job finding it. But you were all too nice for EvilCrap IMO, he seems like a jerk who shouldn''t be helped so

Share this post


Link to post
Share on other sites
Another thing I noticed is that you never call your alignCopy() in main(). Therefore new never happens, so when you delete [], the explosion occurs.

The delete [] in the destructor relies on the fact that alignCopy() was called. You''re just asking for trouble with this.

Share this post


Link to post
Share on other sites
quote:
Original post by Anonymous Poster
[quote]Original post by Gorg
Look at compare, you are passing by value, but you do not have a copy constructor that makes a copy of the internal string, so after your call to compare, fstr2 internal strings points to crap, because the compare FastString points to same data as fstr2 data.
Wow, great job finding it. But you were all too nice for EvilCrap IMO, he seems like a jerk who shouldn''t be helped so



Because of his name I thought he was role playing

But even though I find him very aggressive in some of his replies, he really did got some less than usefull replies. I mean, if someone does not want to read his code and not help him, just don''t post.


Share this post


Link to post
Share on other sites
quote:
Original post by Gorg
But even though I find him very aggressive in some of his replies, he really did got some less than usefull replies. I mean, if someone does not want to read his code and not help him, just don''t post.


I didn''t need to read his code to know that his approach was wrong. By his own admission, he had spent around a week debugging this code due to making what turned out to be a beginner''s mistake. Would you rather spend a week debugging a piece of code which is already available in the C++ Standard Library?

--
1st law of programming: Any given program, when running, is obsolete.

Share this post


Link to post
Share on other sites
quote:
Original post by SabreMan
[quote]Original post by Gorg
But even though I find him very aggressive in some of his replies, he really did got some less than usefull replies. I mean, if someone does not want to read his code and not help him, just don't post.


I didn't need to read his code to know that his approach was wrong. By his own admission, he had spent around a week debugging this code due to making what turned out to be a beginner's mistake. Would you rather spend a week debugging a piece of code which is already available in the C++ Standard Library?

--
1st law of programming: Any given program, when running, is obsolete.


No I would not. Comparing 2 strings has never been a bottleneck in any of my projects.

I don't really care if he is doing it the right way or not. It took me 2 minutes to find the problem. I only debug those problem because it gives me more insight into C++. ( in this case I gained nothing, but that's life)

My point really was : If you don't want to check the code, don't answer. I should not have said that the other posts were less than usefull because I agree 100% with what you and Olussey said.












Edited by - Gorg on February 19, 2002 7:25:49 PM

Share this post


Link to post
Share on other sites
quote:
Original post by Gorg
My point really was : If you don''t want to check the code, don''t answer.


Why not? There are more potential answers to the question than simply finding the immediate problem. I was looking at the bigger picture, and passing on advice which I believe would gain far more benefits than fixing this one mistake. I''ve met many people who have been grateful for being shown how to raise their productivity levels, and I occasionally meet a few who are completely ungrateful.

If EvilCrap didn''t want to take the advice he could have ignored it instead of having a childish tantrum. Sure, he can call me a "jackass", but you won''t find me around here asking for help on a simple programming problem that''s taken days out of my development schedule.

--
The placement of a donkey''s eyes in its head enables it to see all four feet at all times.

Share this post


Link to post
Share on other sites
actually, i had byref in there earlier but removed it in a brain fart while fixing a different bug.

i fixed the class earlier yesterday WITHOUT ANY OF UR HELP.

but thanks alot gorg for actually attemptng to be a helpful, and a remotely intelligent person.

although i am a jackass, so are you, saberman. why dont u get slightly humble, you self involved... -




Edited by - evilcrap on February 20, 2002 1:04:08 PM

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
quote:
Original post by EvilCrap
i fixed the class earlier yesterday WITHOUT ANY OF UR HELP.
HAHAHA.. yeah, right. We all believe that

At least you managed to live up to your name to the very end.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
quote:
Original post by EvilCrap
omfg, please dont respond if you dont know what ur talking about.

string.h is for string ops on char*.
string is so i can write a std string set function, or template.

this unoptimized class is significantly faster than std string, which was the intention.

the compare works perfectly

[quote]
I think the error lies in the allocation and copy wrangling used to transfer char * data into other types.


that may be, but my tests say otherwise.



anyone: please dont respond if you dont know what your talking about, or dont bother to actually skim the source.

After uncommenting the destructor and passing the string as a ref, I have no problems running the code.

BTW, you can call it unit testing all you want, but I just don''t trust a programmer that sets a variable to null then doesn''t check it before deleting.

Share this post


Link to post
Share on other sites
omfg
there are implementation problems

this is more proper align source:
  
if( _pStringStart != NULL) delete[] _pStringStart;

//do not align or if NULL

if (pCstr != NULL)
{
//align legnth to sizeof(myType)

int length = strlen(pCstr);
int alignedLength = (length + sizeof(myType) - length % sizeof(myType) ) / sizeof(myType);

//set up pointers

_pStringStart = new myType[alignedLength];
_pStringEnd = _pStringStart + alignedLength;

//set last sizeof(myType) bytes to NULL for safety

_pStringStart[alignedLength-1] = 0;

//copy to string

strcpy((char*) _pStringStart, pCstr);
}
else
{ //set to NULL so no errors in compare function

_pStringStart = NULL;
_pStringEnd = NULL;
}

Share this post


Link to post
Share on other sites