[ryan20fun]

Hi All.

i dont know why this is showing up as a mempry leak.

HRESULT NarrowString( const wchar_t* String, char** dest )
{
	if( String == TNULL )
		return E_INVALIDARG;

	char* temp = TNULL;

	int lenW = lstrlenW( String );
	int lenA = ::WideCharToMultiByte( CP_ACP, 0, String, lenW, TNULL, 0, TNULL, nullptr );
	if ( lenA > 0)
	{
		temp = TNEW char[ lenA + 1 ]; // allocate a final null terminator as well
		::WideCharToMultiByte( CP_ACP, 0, String, lenW, temp, lenA, TNULL, TNULL );
		temp[ lenA ] = 0; // Set the null terminator yourself
	}
	else
	{
		// handle the error
		DWORD dwError = GetLastError();
		return HRESULT_FROM_WIN32( dwError );
	}

	*dest = TNEW char[ lenA  + 1 ];
	memmove( *dest, temp, lenA + 1 );
	temp = TNULL;

	return S_OK;
}

also why do does this only work if "dest" is a double pointer ?
because when i try it as a single pointer "dest" is shown up as a bad pointer.

the reason why i dont know why this is causing the leak is because i append the resulting char array to a std::string.

this is how im appending the string

HRESULT NarrowString( const std::wstring* String, std::string* dest )
{
	if( String == nullptr )
		return E_INVALIDARG;

	char* narrow = TNULL;
	HRESULT hr = TIPNarrowString( String->c_str(), &narrow );
	if( FAILED( hr ) )
		return hr;

	dest->clear();
	dest->append( narrow );

	return S_OK;
}

Thanks In Advance.

This post has been edited by ryan20fun: 10 November 2011 - 05:38 AM

[CTphpnwb]

memmove simply copies data. It does not remove dest so when you call the function again the next dest adds to the memory you're using. Try putting a delete at the end of the function:

delete dest;

[Salem_c]

It looks more like temp = TNULL; is the leak.

And it would be delete [ ] temp; since you used [ ] in the allocation to begin with.

[ryan20fun]

CTphpnwb, on 10 November 2011 - 06:03 AM, said:

memmove simply copies data. It does not remove dest so when you call the function again the next dest adds to the memory you're using. Try putting a delete at the end of the function:

delete dest;

ok, i thought that memmove would move the memory.
will try that

Salem_c, on 10 November 2011 - 11:19 AM, said:

It looks more like temp = TNULL; is the leak.

TNULL is just #define TNULL nullptr

Salem_c, on 10 November 2011 - 11:19 AM, said:

And it would be delete [ ] temp; since you used [ ] in the allocation to begin with.

ok will try that

so i seams that memmove does not do what i though it did( move the data ).
would it be more logical to use memcpy instead and then delete temp ?
also would it be highly improberble that temp and dest overlap since i create the one in the function ?

[Salem_c]

temp and *dest are the result of two separate calls to new, so they (by definition) do NOT overlap.

For non-overlapping memory, memmove() and memcpy() do the same thing.

[ryan20fun]

ok less memory leaks.
just "dest" showing up as last leak.
why is the string not taking ownership of the char array ?

[CTphpnwb]

Why do you think we can see your current code?

[ryan20fun]

CTphpnwb, on 11 November 2011 - 06:15 AM, said:

Why do you think we can see your current code?

because anybody can view this page.
if this is in connection with my other thread about private source code backup.
i meant it as a whole.

[CTphpnwb]

So you're saying you've made no changes to the above code but are now getting different results?

[ryan20fun]

CTphpnwb, on 11 November 2011 - 08:56 AM, said:

So you're saying you've made no changes to the above code but are now getting different results?

sorry i should have said that there were two leaks being shown.
deleting temp solves one of them, the other is dest.
what i did was have the dest param get appended to a std::string so that i would not have to worry about deleting the memory right now, but it is not working.

the string does not take ownership of the resulting char array.

[CTphpnwb]

Strings are not arrays. Why would you think that a string could "own" an array?

[ryan20fun]

CTphpnwb, on 11 November 2011 - 09:36 AM, said:

Strings are not arrays.

isnt std::basic_string's an arrays ?

CTphpnwb, on 11 November 2011 - 09:36 AM, said:

Why would you think that a string could "own" an array?

what i want is the string to take care of cleaning up "dest".

[CTphpnwb]

So what would this be then?

string example[10];

Would that be an array of arrays? It looks to me like an array of strings.

Why can't dest clean up after dest?


I have no idea what your current code looks like, so I can only offer generalities.

[Salem_c]

Personally, I think you would be much better off if you stopped messing with memory allocation (and subsequently screwing it up with memory leaks), and just went straight to std::string at the earliest possible moment.

HRESULT NarrowString( const wchar_t* String, std::string &dest )
{
        dest = "";  // nothing yet

	if( String == TNULL )
		return E_INVALIDARG;

	char* temp = TNULL;

	int lenW = lstrlenW( String );
	int lenA = ::WideCharToMultiByte( CP_ACP, 0, String, lenW, TNULL, 0, TNULL, nullptr );
	if ( lenA > 0)
	{
		temp = TNEW char[ lenA + 1 ]; // allocate a final null terminator as well
		::WideCharToMultiByte( CP_ACP, 0, String, lenW, temp, lenA, TNULL, TNULL );
		temp[ lenA ] = 0; // Set the null terminator yourself
		dest = temp;  // temp lasts as long as necessary, and no more
        	delete [ ] temp;
	}
	else
	{
		// handle the error
		DWORD dwError = GetLastError();
		return HRESULT_FROM_WIN32( dwError );
	}

	return S_OK;
}

[ryan20fun]

Thanks All!

the memory leak is gone