Praful Srivastava
Praful Srivastava

Reputation: 1

why isn't CreateProcessW() performing the Command provided?

To present the minimal reproducible code I wrote a code to delete a file from a given location using CreateProcessW(). The file does not get deleted. Some help would be really useful in knowing why this isn't working.

dprintf(("Error %d", GetLastError()));
STARTUPINFO si = { sizeof(STARTUPINFO), 0 };
si.cb = sizeof(si);
PROCESS_INFORMATION pi = { 0 };
LPWSTR AppName = L"C:\\Windows\\System32\\cmd.exe";
string bstr = "C:\\Windows\\System32\\cmd.exe /C del"+trans_loc+"a.rtf";
LPWSTR Command = new WCHAR[bstr.length()];
int wchars_num = MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, NULL, 0);
MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, Command, wchars_num);
DWORD res = CreateProcessW(AppName, Command, 0, 0, 0, DETACHED_PROCESS, 0, 0, &si, &pi);

WaitForSingleObject(pi.hProcess, INFINITE);

define TRANSCRIPT_LOCATION "C:\Users\Administrator\Desktop\" this is the location of the file to be deleted

GetLastError() keeps returning 50(ERROR_NOT_SUPPORTED) and the value of res = 1

Upvotes: 0

Views: 383

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 596186

I see a number of problems with your code:

  • LPWSTR AppName = L"C:\\Windows\\System32\\cmd.exe"; does not compile in C++11 and later. You need to (and should) use LPCWSTR instead, since a string literal is const data, and LPCWSTR is a pointer to const WCHAR data, but LPWSTR is a pointer to non-const WCHAR data.

  • In string bstr = "C:\\Windows\\System32\\cmd.exe /C del"+trans_loc+"a.rtf";, you are missing a required space character between the del command and the filename to delete.

  • In LPWSTR Command = new WCHAR[bstr.length()];, you are not allocating enough space for a null terminator. Also, you should not be using bstr.length() for the converted length anyway, because there is no guarantee that the converted string will not be larger than the original string. You should call MultiByteToWideChar() one time with a NULL output buffer to calculate the actual converted length (which you ARE doing), THEN allocate the memory (which you are NOT doing - you are allocating too soon!), THEN call MultiByteToWideChar() again to do the actual conversion.

  • You are leaking the allocated memory (you are not calling delete[] Command;). I would suggest using std::wstring or std::vector<WCHAR> instead of new WCHAR[].

  • You say that res is being set to 1, which means CreateProcessW() is actually successful in running cmd.exe (now, whether cmd.exe is successful in executing your command is a different matter - use GetExitCodeProcess() to find that out), and thus the return value of GetLastError() is meaningless! It is certainly meaningful to call GetLastError() before calling CreateProcessW()

  • You are calling WaitForSingleObject() regardless of whether CreateProcessW() succeeds or fails.

Try this instead:

STARTUPINFO si = {};
si.cb = sizeof(si);
PROCESS_INFORMATION pi = {};

std::string bstr = "C:\\Windows\\System32\\cmd.exe /C del \"" + trans_loc + "a.rtf\"";

int wchars_num = MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), bstr.length(), NULL, 0);
if (wchars_num == 0)
{
    dprintf(("MultiByteToWideChar Error %d", GetLastError()));
}
else
{
    std::vector<WCHAR> Command(wchars_num + 1);
    MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), bstr.length(), Command.data(), wchars_num);

    if (!CreateProcessW(nullptr, Command.data(), nullptr, nullptr, FALSE, DETACHED_PROCESS, nullptr, nullptr, &si, &pi))
    {
        dprintf(("CreateProcessW Error %d", GetLastError()));
    }
    else
    {
        WaitForSingleObject(pi.hProcess, INFINITE);

        DWORD dwExitCode = 0;
        GetExitCodeProcess(pi.hProcess, &dwExitCode);
        dprintf(("cmd.exe Exit Code %d", dwExitCode));

        CloseHandle(pi.hThread);
        CloseHandle(pi.hProcess);
    }
}

Or, if you are using Windows 10 build 17035 or later and have enabled the "Beta: Use Unicode UTF-8 for worldwide language support" option in your Windows settings (or, if trans_loc does not contain any non-ASCII, non-user-locale characters), then no MultiByteToWideChar() conversion is needed at all:

STARTUPINFO si = {};
si.cb = sizeof(si);
PROCESS_INFORMATION pi = {};

std::string Command = "C:\\Windows\\System32\\cmd.exe /C del \"" + trans_loc + "a.rtf\"";

if (!CreateProcessA(nullptr, const_cast<char*>(Command.c_str()), nullptr, nullptr, FALSE, DETACHED_PROCESS, nullptr, nullptr, &si, &pi))
{
    dprintf(("CreateProcessA Error %d", GetLastError()));
}
else
{
    WaitForSingleObject(pi.hProcess, INFINITE);

    DWORD dwExitCode = 0;
    GetExitCodeProcess(pi.hProcess, &dwExitCode);
    dprintf(("cmd.exe Exit Code %d", dwExitCode));

    CloseHandle(pi.hThread);
    CloseHandle(pi.hProcess);
}

That being said, a better option would be to simply use std::wstring instead of std::string to begin with:

STARTUPINFO si = {};
si.cb = sizeof(si);
PROCESS_INFORMATION pi = {};

// make sure trans_loc is std::wstring instead of std::string...
std::wstring bstr = L"C:\\Windows\\System32\\cmd.exe /C del \"" + trans_loc + L"a.rtf\"";

if (!CreateProcessW(nullptr, Command.data(), nullptr, nullptr, FALSE, DETACHED_PROCESS, nullptr, nullptr, &si, &pi))
{
    dprintf(("CreateProcessW Error %d", GetLastError()));
}
else
{
    WaitForSingleObject(pi.hProcess, INFINITE);

    DWORD dwExitCode = 0;
    GetExitCodeProcess(pi.hProcess, &dwExitCode);
    dprintf(("cmd.exe Exit Code %d", dwExitCode));

    CloseHandle(pi.hThread);
    CloseHandle(pi.hProcess);
}

Of course, the simplest solution would be to just not use cmd.exe / C del at all, but instead use DeleteFileW():

// make sure trans_loc is std::wstring instead of std::string...
std::wstring bstr = trans_loc + L"a.rtf";

if (!DeleteFileW(bstr.c_str()))
{
    dprintf(("DeleteFileW Error %d", GetLastError()));
}

Or, if you insist on using a UTF-8 encoded std::string:

std::string bstr = trans_loc + "a.rtf";

int wchars_num = MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), bstr.length(), NULL, 0);
if (wchars_num == 0)
{
    dprintf(("MultiByteToWideChar Error %d", GetLastError()));
}
else
{
    std::vector<WCHAR> wstr(wchars_num + 1);
    MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), bstr.length(), wstr.data(), wchars_num);

    if (!DeleteFileW(wstr.c_str()))
    {
        dprintf(("DeleteFileW Error %d", GetLastError()));
    }
}

Or, if you are using Windows 10 with UTF-8 support enabled (or, if trans_loc does not contain any non-ASCII, non-user-locale characters):

std::string bstr = trans_loc + "a.rtf";

if (!DeleteFileA(bstr.c_str()))
{
    dprintf(("DeleteFileA Error %d", GetLastError()));
}

Upvotes: 1

R Sahu
R Sahu

Reputation: 206577

My first thought is that

LPWSTR Command = new WCHAR[bstr.length()];

is not right. Perhaps

LPWSTR Command = new WCHAR[bstr.length() + 1];

will work. A better alternative is to use wchars_num for allocating memory.

instead of

LPWSTR Command = new WCHAR[bstr.length()];
int wchars_num = MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, NULL, 0);
MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, Command, wchars_num);
DWORD res = CreateProcessW(AppName, Command, 0, 0, 0, DETACHED_PROCESS, 0, 0, &si, &pi);

use

int wchars_num = MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, NULL, 0);
LPWSTR Command = new WCHAR[wchars_num];
MultiByteToWideChar(CP_UTF8, 0, bstr.c_str(), -1, Command, wchars_num);
DWORD res = CreateProcessW(AppName, Command, 0, 0, 0, DETACHED_PROCESS, 0, 0, &si, &pi);

A second issue is that perhaps you missed a space character when composing the del command.

string bstr = "C:\\Windows\\System32\\cmd.exe /C del " + trans_loc + "a.rtf";
//                                                  ^^

Upvotes: 2

Related Questions