Sign in to follow this  
guillermoro

Mem Leaks

Recommended Posts

guillermoro    109
is there a memory leak or some other error in this, it compiles but im not sure
string cTimer::getFormattedTimeFromStart()
{
	//Returns time from start in:	h : m : s
	int hours,mins,secs;
	secs = (int)(getTime() - _startTime) / 1000;
	hours = (int)(getTime() - _startTime) / 1000 / 60 / 60;
	mins = (int) (getTime() - _startTime) / 1000 / 60;
	string str1;
	string str2;
	string str3;
	char* secsBuffer = new char[2];
	char* minsBuffer = new char[2];
	char* hoursBuffer = new char[2];
	itoa(secs,secsBuffer,10);
	itoa(secs,minsBuffer,10);
	itoa(secs,hoursBuffer,10);
	str1.assign(secsBuffer);
	str2.assign(minsBuffer);
	str3.assign(hoursBuffer);
	str3.append(new char(':'));
	str3.append(str2);
	str3.append(new char(':'));
	str3.append(str1);
	delete[] secsBuffer;
	delete[] minsBuffer;
	delete[] hoursBuffer;
	return str1;
}


I included string.h and a few other files but i dont think that matters because it does compile(even though itoa is deprecated). Im also not sure if it will work, i want it to return the time from the start in hours mins and secs in one string. _startTime is the time in milliseconds from the time the computer turned on until the app started, it is a DWORD. Thanks :-)

Share this post


Link to post
Share on other sites
jyk    2094
Just use std::stringstream instead; it will avoid many potential problems.

As far as your current code goes though, the buffers have local scope and small fixed sizes, so you shouldn't be allocating them dynamically anyway.

Share this post


Link to post
Share on other sites
Ro_Akira    212
I'm would imagine the str3.append(new char(':')); and similar would leak. Why not just use str3.append(':'); ?

As jyk said, use stack allocated objects if they're not too big (1MB stack limit a typical default on Win32 I think?). It has the added bonus of being more efficient, as well as less error prone.

Note also that itoa should be provided with a large enough buffer. about itoa. Writing past the end of buffers is not good. Using std::stringstream would avoid such issues.

Share this post


Link to post
Share on other sites
RDragon1    1205

{
//Returns time from start in: h : m : s
int hours,mins,secs;
secs = (int)(getTime() - _startTime) / 1000;
hours = (int)(getTime() - _startTime) / 1000 / 60 / 60;
mins = (int) (getTime() - _startTime) / 1000 / 60;

stringstream ss;
ss << hours << ':' << mins << ':' << secs;
return ss.str();
}


Share this post


Link to post
Share on other sites
guillermoro    109
:-(
i just tried that and it seems i get these errors

.\Timer.cpp(35) : error C2079: 'ss' uses undefined class 'std::basic_stringstream<_Elem,_Traits,_Alloc>'
with
[
_Elem=char,
_Traits=std::char_traits<char>,
_Alloc=std::allocator<char>
]
.\Timer.cpp(36) : warning C4293: '<<' : shift count negative or too big, undefined behavior
.\Timer.cpp(36) : warning C4293: '<<' : shift count negative or too big, undefined behavior
.\Timer.cpp(36) : warning C4552: '<<' : operator has no effect; expected operator with side-effect
.\Timer.cpp(37) : error C2228: left of '.str' must have class/struct/union
type is 'int'

Share this post


Link to post
Share on other sites
Mike2343    1202
Just off the top of my head, you're including the correct headers for stringstream? You're also, I assume using namespace std in the .cpp file? I don't have a compiler handy to test this right now, so just ideas off the top of my head. Looks like it should work though.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this