GetProcessId returns parrent instead of needed

Question

This function should return correct process id. It but if process has parrent process - it returns process id of it's parrent. Why? How to fix it?

DWORD _getProcId(LPCSTR processName)
{
    PROCESSENTRY32 entry;
    entry.dwSize = sizeof(PROCESSENTRY32);

    HANDLE snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, NULL);
    if (Process32First(snapshot, &entry) == TRUE)
    {
        while (Process32Next(snapshot, &entry) == TRUE)

        {
            if (_stricmp(entry.szExeFile, processName) == 0)
            {
                HANDLE h = OpenProcess(PROCESS_ALL_ACCESS, false, entry.th32ProcessID);
                if (h)
                {
                    return GetProcessId(h);
                }
                else
                {
                    CloseHandle(snapshot);
                    return 0;
                }
            }
        }
    }

    CloseHandle(snapshot);
    return 0;
}

What returns what, what are you asking? Your question seems a bit confused. — Jesper Juhl, Jul 23 '18 at 19:56
This function. It should return id of process found by name. But if found process is child process of another process - this function returns ID of it's parent process instead of found process. — Georgiy List'ev, Jul 23 '18 at 19:59
Why call OpenProcess, then GetProcessId just to return the id of the process when you already have `entry.th32ProcessID`? Also, is it possible that you have multiple processes running with the same exefile? Becuase your code performs a _linear search_ and I expect it to return the **first** match (which is likely the parent...assuming they are "forked", i.e.: both parent and child are executing the same .exe). Also you leak the handle `h` returned by OpenProcess. — Wyck, Jul 23 '18 at 20:08
Unrelated: the _ in `_getProcId` could bite you. More on that: [What are the rules about using an underscore in a C++ identifier?](https://stackoverflow.com/questions/228783/what-are-the-rules-about-using-an-underscore-in-a-c-identifier) — user4581301, Jul 23 '18 at 20:08
I think `Process32Next` should be called after the loop body, otherwise you skip the first entry. — user7860670, Jul 23 '18 at 20:09
`return GetProcessId(OpenProcess(..., entry.th32ProcessID));` is a convoluted way of writing `return entry.th32ProcessID;`. If nothing else, that solves the problem you seem to be having with deciding, which access you need. You *certainly* don't need `PROCESS_ALL_ACCESS`. And it doesn't expose a handle leak, like your code does. — IInspectable, Jul 23 '18 at 21:55

score 2 · Answer 1 · answered Jul 24 '18 at 03:24

Your Get Process ID function will return the first process with the matching executable filename. That is why it's returning the parent process, because it's the first one to match the string comparison. If you have 2 running processes with the same name this will happen. Not much you can do about that.

Also here is some slightly better code (using unicode)

DWORD GetProcId(const wchar_t* targetProcess)
{
    DWORD procId = 0;
    HANDLE hSnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
    if (hSnap != INVALID_HANDLE_VALUE)
    {
        PROCESSENTRY32 procEntry;
        procEntry.dwSize = sizeof(procEntry);
        if (Process32First(hSnap, &procEntry))
        {
            do
            {
                if (!wcscmp(procEntry.szExeFile, targetProcess))
                {
                    procId = procEntry.th32ProcessID;
                    break;
                }
            } while (Process32Next(hSnap, &procEntry));
        }
    }
    CloseHandle(hSnap);
    return procId;
}

This is the documentation from MSDN that has more detail https://learn.microsoft.com/en-us/windows/desktop/ToolHelp/taking-a-snapshot-and-viewing-processes

It makes more sense to call this GetProcId() function and then call OpenProcess afterwords IMO to keep everything separate, easier to read and easier to debug.

GetProcessId returns parrent instead of needed

1 Answers1