views:

42

answers:

1
WCHAR* someString = L"SomeString\n";
WCHAR s1[MAX_PATH];

// I'm sure this is right code.
StringCchCopyW(s1, _countof(s1), someString);

// But I'm not sure about these example.
StringCchCopyW(s1 + 100, _countof(s1) - 100, someString); // Is it right?
StringCchCopyW(s1 + 100, _countof(s1), someString); // Is it right?

// How about these?
StringCchCatW(s1, _countof(s1) - wcslen(s1), someString); // Is it right?
StringCchCatW(s1, _countof(s1), someString); // Is it right?
+1  A: 

One thing that's wrong with all of the functions is you've neglected to check the return value. Even String Safe functions can fail and if they do they will silently in your code. Return values for string functions should always be checked or you've opened yourself to security holes.

Other than that it's really hard to answer this with a definitive yes or no because there are subtle issues to consider. Lets take them in order

StringCchCopyW(s1, _countof(s1), someString);

This is correct and has no real issues other than the ignored return value.

StringCchCopyW(s1 + 100, _countof(s1) - 100, someString); // Is it right?

It's hard to answer this one. Following this operation s1+100 will point to a valid string object but this does nothing for s1. It doesn't open any security holes though.

StringCchCopyW(s1 + 100, _countof(s1), someString); // Is it right? 

This is very wrong. You're stating the number of valid characters is MAX_PATH but passing a buffer which has MAX_PATH-100 characters. This can be buffer overflow'd if someString is long enough.

StringCchCatW(s1, _countof(s1) - wcslen(s1), someString); // Is it right?

This is wrong but not dangerously so. You're reporting that the length of s1 is actually smaller than it is. It won't cause an overflows but it will prevent valid strings from being added.

StringCchCatW(s1, _countof(s1), someString); // Is it right?

This is correct provided that s1 has a valid string in it. If s1 does not have a valid string in it then it can cause the function to incorrectly execute.

JaredPar
It's clear. Thanks.
Benjamin
By the way, you are the one who developed VsVim! It's awesome. Life changing :)
Benjamin
@Benjamin, glad to hear you're enjoying VsVim!
JaredPar
The ue of wcslen(s1) can cause an oerflow if the current contents of the buffer are not null-terminated.
Remy Lebeau - TeamB