1

I used below code to get the ProgramData path using SHGetKnowFolderPath().

Can you please tell me if it is the correct way to use it with CString? If not, what will be the best possible solution to get the ProgramData path using SHGetKnownFolderPath()?

I used the 2 samples below and both work, but I'm not sure whether they are correct.

CString szProgramDataPath;  
szProgramDataPath.GetBuffer(MAX_PATH);  
if (FAILED(SHGetKnownFolderPath(FOLDERID_ProgramData, 0, NULL, (PWSTR*)&szProgramDataPath)))
{
    cout << _T("Failed, error is: ") << GetLastError();
}

Or:

CString szProgramDataPath;  
if (FAILED(SHGetKnownFolderPath(FOLDERID_ProgramData, 0, NULL, (PWSTR*)&szProgramDataPath)))
{
    cout << _T("Failed, error is: ") << GetLastError();
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • When using `GetBuffer()` this way, make sure you call `ReleaseBuffer()` afterwards. – Danny_ds Mar 16 '17 at 19:27
  • @Danny_ds technically, you need to call `ReleaseBuffer()` only if the content of the buffer changes and the new string length is different than before the change. Otherwise calling `ReleaseBuffer()` is optional as it will basically be a no-op. But, it is always good practice to call `ReleaseBuffer()` after `GetBuffer()`, just in case. Maybe someday MS redesigned `GetBuffer()` to allocate a temp memory block and `ReleaseBuffer()` merges it into the main string. Who knows. – Remy Lebeau Mar 16 '17 at 22:21
  • @RemyLebeau Right again :) – Danny_ds Mar 16 '17 at 23:08

3 Answers3

6

Neither approach is correct.

SHGetKnownFolderPath() returns a pointer to a newly allocated string, which you must free with CoTaskMemFree() when finished using it.

Both code examples are attempting to use operator& to gain direct access to CString's internal pointer to its character data, in hopes that the CString will take ownership of the pointer returned by SHGetKnownFolderPath().

This is WRONG and VERY DANGEROUS! For several reasons...

You are assuming that the address of that internal pointer is the same address as the CString object itself. CString does not override operator& to return the address of its internal buffer pointer, like you are assuming it does.

Even if it did, the first code example is still broken because it has a memory leak. You are pre-allocating the character buffer with GetBuffer() (and not calling ReleaseBuffer() afterwards). SHGetKnownFolderPath() will not free that buffer before re-assigning the pointer to point at the returned memory block, so you end up leaking the earlier memory block.

And you are assuming that CStrings destructor uses CoTaskMemFree() (or compatible function) to free its internal character buffer. Which is not safe to assume.

And you are assuming that the memory layout of the CStrings internal character buffer is compatible with the memory layout of the memory block returned by SHGetKnownFolderPath(). That is actually NOT the case, as CString implements a reference counter for its character data (amongst other things). Where do you thing that counter is stored? Right, inside the character buffer itself! Since SHGetKnownFolderPath() does not return a memory buffer that is compatible with CString's data format, you are going to corrupt your CString data and cause problems when it later tries to access things that do not exist in memory. See CString In A Nutshell for more information about `CString's internal details.

In short, CString is not designed to be used the way you are attempting. You are making some very dangerous assumptions, and thus causing undefined behavior in your code!

You cannot directly access the CString's internal buffer pointer to reassign it to a different address. The correct solution would look more like this instead:

CString szProgramDataPath;  
LPWSTR pProgramDataPath;

if (FAILED(SHGetKnownFolderPath(FOLDERID_ProgramData, 0, NULL, &pProgramDataPath)))
{
    // DON'T use _T() here! std::cout expects char* strings only...
    cout << "Failed, error is: " << GetLastError();
}
else
{
    szProgramDataPath = pProgramDataPath;  
    CoTaskMemFree(pProgramDataPath);
}

If you want to automate the destruction of the returned memory block, especially if the CString assignment fails with an exception, use a smart pointer like std::unique_ptr (see std::unique_ptr, deleters and the Win32 API), eg:

CString szProgramDataPath;  
LPWSTR pProgramDataPath;

if (FAILED(SHGetKnownFolderPath(FOLDERID_ProgramData, 0, NULL, &pProgramDataPath)))
{
    // DON'T use _T() here! std::cout expects char* strings only...
    cout << "Failed, error is: " << GetLastError();
}
else
{
    std::unique_ptr<WCHAR, decltype(CoTaskMemFree)> deleter(pProgramDataPath, &CoTaskMemFree);
    szProgramDataPath = pProgramDataPath;  
}
Community
  • 1
  • 1
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Great answer +1, but this is a bit confusing: _"Where do you thing that counter is stored? Right, inside the character buffer itself"_, because the counter (and size etc) is a member of the class, next to a pointer to the internal character buffer. – Danny_ds Mar 16 '17 at 19:39
  • @Danny_ds not according to the "CString in a Nutshell" article I linked to in my answer. Unless Microsoft has redesigned `CString` since that article was written. But moving the reference count (and other data attributes) outside of the data buffer would not make sense, and it makes the whole recounting system and copy constructor/assignment harder to manage. – Remy Lebeau Mar 16 '17 at 22:03
  • You are right (as usual :). I remember reading the source code several times in the past, must have overlooked or have forgoten about this. Thanks for refreshing my mind! – Danny_ds Mar 16 '17 at 23:08
0

Syntax is:

HRESULT SHGetKnownFolderPath(
   _In_     REFKNOWNFOLDERID rfid,
   _In_     DWORD            dwFlags,
   _In_opt_ HANDLE           hToken,
   _Out_    PWSTR            *ppszPath
 );

Note the 4th parameter is a PWSTR* rather than a CString.

From MSDN:

ppszPath [out]
Type: PWSTR*

When this method returns, contains the address of a pointer to a null-terminated Unicode string that specifies the path of the known folder. The calling process is responsible for freeing this resource once it is no longer needed by calling CoTaskMemFree. The returned path does not include a trailing backslash. For example, "C:\Users" is returned rather than "C:\Users\".

What you need is a PWSTR variable to pass in. Then you will need to free the pointed resource afterwards:

LPWSTR path = NULL;  //LP = "Long pointer" -- same as PWSTR *path;  
SHGetKnownFolderPath(&FOLDERID_ProgramData, 0, NULL, &path);

...

CoTaskMemFree(path);

And a CString can be assigned to directly from a the LPWSTR path variable after checking that SHGetKnownFolderPath returned succesfully.

CString szProgramDataPath = path;
jhh
  • 663
  • 4
  • 6
  • What you say is technically true, but doesn't address the question actually asked of how to get the returned path into a `CString` the correct way. – Remy Lebeau Mar 16 '17 at 22:10
0

If i got your question you need to take a look at "mbstowcs" and "wcstombs" functions which convert between wchar* and char* and vise versa

AMS
  • 104
  • 9