wcscpy_s crash

[Patlegroin]

Hi all,

I gotta problem using wcscpy_s(first time using it), I guess I'm just doint it wrong, as said my gf for my other first time.

So, I have a method like this :

CString::CString(wchar_t *text)
{
	TRACE(CTrace::NIV_DEB, "CString(wchar_t *text)");

	const size_t len = wcslen(text);
	TRACE(CTrace::NIV_INFO, len);
	_text = new wchar_t[len+1];
	TRACE(CTrace::NIV_INFO, "B");
	wcscpy_s(_text, len, text);

	TRACE(CTrace::NIV_FIN, "CString(wchar_t *text)");
}

where wchar_t *_text is a member data of the CString class.

The call to this method : CString toto((wchar_t*)"test");

While debugging, when I reach the line wcscpy_s(_text, len, text); I got an error message on the _text allocation

"Error while reading ..." -> <impossible to read memory>

.
So, maybe it's allocation problem, but I can't figure out what's wrong with my code...

You can see this screen shot, I'm not sure it helps much(french language...) : http://imageshack.us/photo/my-images/96/bugscreen2.jpg/

I'm using visual studio 2012.

Thank you in advance ^^

[codewalker]

CString toto((wchar_t*)"test"); should be CString toto(L"test"); or CString toto( _T("test") );

The last one is frequently used in VC.

[Patlegroin]

Thank you codewalker,

I tried both, but it didn't change anything. I guess the problem may come from the _text = new wchar_t[len+1]; since the debugger says it can't read the memory there, right after allocation is done...

Your advice will certainely be useful when my first bug will be solved ^^

[modoran]

CString has constructors for both char and wchar_t, so you can use either of them.

As for wcscpy_s, I never understands how it works and always crashes for me, even a simple example. Use wcscpy, this always works as expected (no, the problem is NOT in the line when you allocate memory).

[Patlegroin]

Thank you Modoran,

I first wrote wcscpy, but it was marked as deprecated so... I changed it into wcscpy_s as advised by visual studio... hm... I will tried that once back home and be careful from who I accept advises :)

[kbw]

It just goes to show how pointless the "safe" functions are.

An alternative is to use the standard function:

wcsncpy(_text, text, len + 1);

Further more, as you've guaranteed that the destination is large enough, you can just use:

wcscpy(_text, text);

[andywestken]

Could you post the relevant part of your class declaration for CString, wcscpy_s shouldn't ever get that upset!

BTW - codewalker did get you to correct CString toto((wchar_t*)"test"); to something more suitable.

But I just wanted to check that you picked up why it's so evil to lie to the compiler, that a multi-byte (or Ansi) string is actually Unicode. The terminator for a Unicode string is two 0 bytes, whereas a multi-byte string ends with just a single one (not enough for Unicode). On top of that, if it did display, it is likely to be some wierd characters from some random code page, rather than normal letters.

[Patlegroin]

Thank you all for caring and helping.

Using wcsncpy(_text, text, len + 1); solved the problem.
Using CString toto((wchar_t*)"test"); was effectively not a correct manner to write things. It doesn't crash but I get unexpected caracters in some situations.
Using CString toto( _T("test") ); solve this problem.
@Andywestken : I even tried to write a \0\0 at the end of the string, but it didnt help... :)
Anyway, as you pointed, it's not a correct way to write things.

I'm not sure what you call the relevant part of the class CString, so... I give it all. Anyway, so short...

// string.cpp
#include "string.h"
#include <windows.h>
#include <wchar.h>
#include "trace.h"

CString::CString()
{
	_text=nullptr;
}

CString::CString(wchar_t *text)
{
	const size_t len = wcslen(text);
	_text = new wchar_t[len+1];
	wcsncpy(_text, text, len + 1); // <- it works
	// wcscpy_s(_text, len, text); // <- it doesn't...
}

CString::~CString()
{
	erase();
}


wchar_t *CString::get()
{
	return _text;
}

void CString::set(wchar_t *text)
{
	erase();
	const size_t len = wcslen(text);
	_text = new wchar_t[len];
	wcscpy_s(_text, len, text);
}

void CString::erase()
{
	if(nullptr!=_text) delete _text;
	_text=0;
}

Is it what you expected ?

Thank all again ^^

[andywestken]

The line should be

wcscpy_s(_text, len + 1, text);

The second parameter is the buffer length, not the string length. When I run your code, with just len, I get an assertion: "Buffer is too small"."

Basically, it's complaining that there's no room for it to add the null terminator it wants to add. (You don't get the assert because you're building the release target, going by your picture.)

Andy

PS Writing 2 \0s at the end of a Ansi string doesn't make it Unicode:
wchar_t* -> this string is using 2 bytes per char
char* -> this string is using 1 byte per char (except in special cases)

[Patlegroin]

I saw the "Buffer is too small" in debug, but I couldn't get rid of it.
I tried to allocate a buffer with 1000 wchar_t, then copy "len" length. Still same problem.
So... I still can't make the wcscpy_s works, and since the wcsncpy works fine(that means : works as expected, where wcscpy_s doesn't !) I really don't want to know what's going on behind.
If you're interrested, I can send you the whole VS project.

Thank you much for your help.