GetModuleFileNameEx, Access Denied Error

Question

I'm trying to get the name of all my open processes. This is what I have:

   #include "stdafx.h"
#include <Psapi.h>

int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, PSTR pCmdLine, int iCmdShow)
{
    bool _result;
    DWORD *pProcessIds = new DWORD[1000];
    DWORD cb;
    DWORD pBytesReturned;

    _result =  EnumProcesses(pProcessIds, 1000, &pBytesReturned);

    HANDLE hndProccesse;


    for (int i = 0; i < pBytesReturned / sizeof(DWORD); ++i)
    {
        hndProccesse =   OpenProcess(STANDARD_RIGHTS_ALL,  false, *pProcessIds);
        DWORD _len;

        DWORD _len2 =0;
        LPWSTR lpFilename = new WCHAR[100];
        _len =GetModuleFileNameEx(hndProccesse,NULL, lpFilename, _len2);
        DWORD _errr;
        _errr =  GetLastError();
        MessageBox(NULL, lpFilename, NULL, 0);
        CloseHandle(hndProccesse);

        pProcessIds ++;



    }

    return 0;
}

Everything is working fine upto GetModuleFileNameEx which is giving an Access Denied Error (5).

Also this is whats displaying on the message box:

Any Ideas?

Answer 1

The fourth argument to GetModuleFileNameEx() has to be the size of the array passed in the third argument. You pass in _len2. But you set _len2 to zero, not to the size of your lpFilename array (100). So GetModuleFileNameEx() sees it has nothing to do, and doesn't even touch your lpFilename array. Heap data isn't necessarily initialized to zeroes, so lpFilename still contains random garbage, hence the random message box contents.

I am going to guess that GetModuleFileNameEx() returned zero because it didn't need to write anything, but didn't set the last error code because nothing failed, so the access denied error is left over from an earlier part of the program.

Some more notes:

Keep in mind what Jonathan Potter said about the proper way to check error returns from Windows API functions. You have you return value from GetModuleFileNameEx() in _len. MSDN says GetModuleFileNameEx() returns zero on error. So you need to check _len to see if it equals zero before getting the last error value, otherwise it won't be meaningful. As mentioned earlier, GetModuleFileNameEx() doesn't have to clear the last error value if it succeeds.

HANDLE hndProccesse = new HANDLE; is definitely wrong, but is not a bug in your program (it is a memory leak, though!). HANDLE itself is a pointer, which is why the new was allowed to run. But doing so is meaningless, as HANDLEs are returned by the operating system and generally shouldn't be used as pointers. Consider them opaque values instead.

On the subject of memory leaks, you never delete[] each of the lpFilenames you create in your loop, nor do you delete[] pProcessIds. This might not be important for the small program you posted above, but if your program ever grows you'll definitely want to fix that.

In general, use MAX_PATH as the nominal length of a filename buffer instead of 100. This is what the various shell functions use. (Longer won't hurt either, but shorter will.)

Answer 2

You need to use FormatMessage to get a useful error description. An example is shown below.. This example is exactly like yours but all it does is incorporate error checking. IT DOES NOT SOLVE THE PROBLEM!

#include <windows.h>
#include <Psapi.h>

void ShowError(DWORD err)
{
    LPTSTR lpMsgBuf = nullptr;
    FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), reinterpret_cast<LPTSTR>(&lpMsgBuf), 0, nullptr);
    MessageBoxA(NULL, lpMsgBuf, "ERROR", 0);
    LocalFree(lpMsgBuf);
}

int main()
{
    DWORD* pProcessIds = new DWORD[1000];
    DWORD pBytesReturned = 0;

    bool res =  EnumProcesses(&pProcessIds[0], 1000, &pBytesReturned);

    if (!res)
    {
        DWORD err = GetLastError();
        MessageBoxW(NULL, L"ENUMPROCESSES FAILED", L"Error", 0);
        ShowError(err);
        delete[] pProcessIds;
        return EXIT_FAILURE;
    }

    for (unsigned int i = 0; i < pBytesReturned / sizeof(DWORD); ++i)
    {
        if (pProcessIds[i] == 0) //error.. process id is 0..
            continue;

        wchar_t lpFilename[256] = {0};
        HANDLE hndProccess =   OpenProcess(STANDARD_RIGHTS_ALL,  false, pProcessIds[i]);

        if (hndProccess == NULL || hndProccess == INVALID_HANDLE_VALUE)
        {
            DWORD err = GetLastError();
            MessageBoxW(NULL, L"FAILED TO OPEN PROCESS", L"ERROR", 0);
            ShowError(err);
            delete[] pProcessIds;
            return EXIT_FAILURE;
        }

        int len = GetModuleFileNameExW(hndProccess, NULL, lpFilename, sizeof(lpFilename) / sizeof(lpFilename[0]));
        if (len <= 0)
        {
            DWORD err = GetLastError();
            if (err)
            {
                MessageBoxW(NULL, L"FAILED TO GET MODULEFILENAME", L"ERROR", 0);
                ShowError(err);
                delete[] pProcessIds;
                return EXIT_FAILURE;
            }
        }          

        CloseHandle(hndProccess);

        MessageBoxW(NULL, lpFilename, L"NAME", 0);
    }

    delete[] pProcessIds;
    return 0;
}