Search code examples
c++concatenationwchar-t

wsprintf() crashes application but is successful


Why on the following code wsprintf() crashes application even if the operation was successful?

#include "Shlwapi.h"
#include <windows.h>
#include <shlobj.h>
#include <iostream>

#pragma comment( lib, "shlwapi.lib")

using namespace std;

int main()
{
    wchar_t buffer[MAX_PATH];
    GetModuleFileName(NULL, buffer, MAX_PATH);

    wchar_t* filepart = PathFindFileName(buffer);
    PathRemoveExtension(filepart);

    PWSTR path = NULL;
    HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);

    wsprintf(path, TEXT("%s\\%s.bz2"), path, filepart);

    wcout << path << endl << endl;

    if (SUCCEEDED(hres))
        CoTaskMemFree(path);

    system("pause");
}

Solution

  • You are trying to write the output of wsprintf() to a memory buffer that is not large enough to hold the output. You are using the buffer that is allocated by SHGetKnownFolderPath(), which is large enough to hold only the path it returns. You can't append to that buffer as-is, you wold have to reallocate it with CoTaskMemRealloc() to make it larger, eg:

    #include "Shlwapi.h"
    #include <windows.h>
    #include <shlobj.h>
    #include <iostream>
    
    #pragma comment( lib, "shlwapi.lib")
    
    using namespace std;
    
    int main()
    {
        wchar_t buffer[MAX_PATH];
        GetModuleFileName(NULL, buffer, MAX_PATH);
    
        wchar_t* filepart = PathFindFileName(buffer);
        PathRemoveExtension(filepart);
    
        PWSTR path = NULL;
        HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);
    
        if (SUCCEEDED(hres))
        {
            int len = lstrlenW(path);
            PWSTR newpath = (PWSTR) CoTaskMemRealloc(path, len + lstrlenW(filepart) + 6);
            if (newpath)
            {
                path = newpath;
                wsprintfW(path+len, TEXT("\\%s.bz2"), filepart);
                wcout << path << endl << endl;
            }
            else
                wcout << L"CoTaskMemRealloc failed" << endl << endl;
    
            CoTaskMemFree(path);
        }
        else
            wcout << L"SHGetKnownFolderPath failed. Error " << (int)hres << endl << endl;
    
        system("pause");
    }
    

    Though really, you should just allocate your own buffer instead and copy the various substrings into it, eg:

    #include "Shlwapi.h"
    #include <windows.h>
    #include <shlobj.h>
    #include <iostream>
    
    #pragma comment( lib, "shlwapi.lib")
    
    using namespace std;
    
    int main()
    {
        wchar_t buffer[MAX_PATH];
        GetModuleFileNameW(NULL, buffer, MAX_PATH);
    
        wchar_t* filepart = PathFindFileNameW(buffer);
        PathRemoveExtensionW(filepart);
    
        PWSTR path = NULL;
        HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);
    
        if (SUCCEEDED(hres))
        {
            wchar_t fullPath = new wchar_t[lstrlenW(path) + lstrlenW(filepart) + 6];
            wsprintfW(fullPath, L"%s\\%s.bz2", path, filepart);
    
            wcout << fullPath << endl << endl;
    
            delete[] fullPath;
            CoTaskMemFree(path);
        }
        else
            wcout << L"SHGetKnownFolderPath failed. Error " << (int)hres << endl << endl;
    
        system("pause");
    }
    

    Or, you could just get rid of wsprintf() and print the various substrings directly to std::wcout instead, eg:

    #include "Shlwapi.h"
    #include <windows.h>
    #include <shlobj.h>
    #include <iostream>
    
    #pragma comment( lib, "shlwapi.lib")
    
    using namespace std;
    
    int main()
    {
        wchar_t buffer[MAX_PATH];
        GetModuleFileNameW(NULL, buffer, MAX_PATH);
    
        wchar_t* filepart = PathFindFileNameW(buffer);
        PathRemoveExtensionW(filepart);
    
        PWSTR path = NULL;
        HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);
    
        if (SUCCEEDED(hres))
        {
            wcout << path << L"\\" << filepart << L".bz2" << endl << endl;
            CoTaskMemFree(path);
        }
        else
            wcout << L"SHGetKnownFolderPath failed. Error " << (int)hres << endl << endl;
    
        system("pause");
    }
    

    If you need a formatted variable, use a std::wstring, such as from std::wostringstream, eg:

    #include "Shlwapi.h"
    #include <windows.h>
    #include <shlobj.h>
    #include <iostream>
    #include <string>
    #include <sstream>
    
    #pragma comment( lib, "shlwapi.lib")
    
    using namespace std;
    
    int main()
    {
        wchar_t buffer[MAX_PATH];
        GetModuleFileNameW(NULL, buffer, MAX_PATH);
    
        wchar_t* filepart = PathFindFileNameW(buffer);
        PathRemoveExtensionW(filepart);
    
        PWSTR path = NULL;
        HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);
    
        if (SUCCEEDED(hres))
        {
            wostringstream oss;
    
            oss << path << L"\\" << filepart << L".bz2";
            CoTaskMemFree(path);
            
            wstring fullPath = oss.str();
            wcout << fullPath << endl << endl;
        }
    
        system("pause");
    }
    

    Alternatively:

    #include "Shlwapi.h"
    #include <windows.h>
    #include <shlobj.h>
    #include <iostream>
    #include <string>
    
    #pragma comment( lib, "shlwapi.lib")
    
    using namespace std;
    
    int main()
    {
        wchar_t buffer[MAX_PATH];
        GetModuleFileNameW(NULL, buffer, MAX_PATH);
    
        wchar_t* filepart = PathFindFileNameW(buffer);
        PathRemoveExtensionW(filepart);
    
        PWSTR path = NULL;
        HRESULT hres = SHGetKnownFolderPath(FOLDERID_Downloads, 0, NULL, &path);
    
        if (SUCCEEDED(hres))
        {
            wstring fullPath;
            fullPath.reserve(lstrlenW(path) + lstrlenW(filepart) + 5);
    
            fullPath += path;
            fullPath += L"\\";
            fullPath += filepart;
            fullPath += L".bz2";
    
            CoTaskMemFree(path);
            
            wcout << fullPath << endl << endl;
        }
    
        system("pause");
    }