Concatenating wide character strings

Question

I just wrote this code to concatenate two wide characters strings. Could someone please comment if it looks OK? (seems to work at least).

SomeStringClass amount = Window->get_text();
wchar_t strAmount[100] = L"Amount: "; // I know I could remove 100 from here too
wchar_t *ptr = wcscat( strAmount, amount.c_str() );
SomeStringClass final;
final.set(ptr, 10 /* 10 is roughly due to length of Amount line 2*/ + amount.length());

Now, final contains what I wanted (e.g. "Amount: 123").

• Yuushi: Why not just use std::wstring?
• user1999360: @Yuushi: Instead of SomeStringClass? (I'll consider that). Just I am programming on a device and it has its own support of strings in the form of SomeStringClass

Answer 1

Firstly, avoid the non-n versions of (w)str_ functions. You've got a buffer overflow just begging to happen here - if amount.length() > 100 - 8, it'll overflow your buffer, causing undefined behaviour and general bad times. Use wcsncat instead:

static const unsigned kstrMax = 100;
wchar_t strAmount[kstrMax] = L"Amount: ";
wchar_t* ptr = wcsncat(strAmount, amount.c_str(), kstrMax - amount.length() - 1);

In fact, we have to be even more careful here, because there's still a bug in the above code (I left it in because I only caught it when modifying my example - it's easy to make mistakes). Say amount.length() > kstrMax, then you'd think the value kstrMax - amount.length() would be below zero, but being unsigned, it will silently overflow, and will instead be some very large positive value, hence we could still easily overwrite the buffer size with this if amount is large enough.

static const unsigned kstrMax = 100;
unsigned currentBufferSize = kstrMax;
wchar_t strAmount[kstrMax] = L"Amount: ";
if(amount.length() >= kstrMax - 8) {
 //find out the difference, remake your buffer and copy across
 currentBufferSize = newBufferSize;
}
wchar_t* ptr = wcsncat(strAmount, amount.c_str(), currentBufferSize - amount.length());

From the looks of it, the amount entered is meant to be a small numeric value. Never assume anything about input. Anything that a user touches must be sanitized before you blindly use it.

std::wstring would make this fiddling about go away:

std::wstring strAmount("Amount: ");
strAmount += amount.c_str();
final.set(strAmount.c_str(), strAmount.size());