C++ Can't get process id (windows)

Question

I have a function like this to get a process' id by it's name, however it always returns 0 with every process i try:

DWORD GetProcessId(std::string ProcessName)
{
HANDLE hsnap;
PROCESSENTRY32 pt;
DWORD PiD;
hsnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
pt.dwSize = sizeof(PROCESSENTRY32);
do {
    if (!strcmp(pt.szExeFile, ProcessName.c_str())) {
        CloseHandle(hsnap);
        PiD = pt.th32ProcessID;
        return PiD;
        if (PiD != NULL) {
            return 0;
        }
    }
} while (Process32Next(hsnap, &pt));
return 1;
}

and main function:

int main()
{
DWORD pid = GetProcessId("calc.exe");
std::cout << pid;
if (pid == 0) { printf("error 1"); getchar(); }//error
return 0;
}

Solution

There are a couple of problems here.

Firstly, GetProcessId is the name of a Windows API function, that takes a single HANDLE as a parameter. HANDLE is usually defined as void* and so what this means is that any pointer will satisfy the function signature.

Your own GetProcessId takes a std::string but you're calling it by passing it a pointer to a string constant. Normally this would be OK, a std::string can be constructed from that, but because the Windows API version of GetProcessId already matches the function signature the compiler calls that in preference. So basically your own function is never actually being called.

There are also a couple of problems with your own function. Firstly, your first iteration through the loop is comparing against garbage memory - you're forgetting to call Process32First, and so pt is uninitialised the first time through. Secondly, if the process isn't found you're leaking the hsnap handle.

Try the following:

DWORD MyGetProcessId(LPCTSTR ProcessName) // non-conflicting function name
{
    PROCESSENTRY32 pt;
    HANDLE hsnap = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
    pt.dwSize = sizeof(PROCESSENTRY32);
    if (Process32First(hsnap, &pt)) { // must call this first
        do {
            if (!lstrcmpi(pt.szExeFile, ProcessName)) {
                CloseHandle(hsnap);
                return pt.th32ProcessID;
            }
        } while (Process32Next(hsnap, &pt));
    }
    CloseHandle(hsnap); // close handle on failure
    return 0;
}

int main()
{
    DWORD pid = MyGetProcessId(TEXT("calc.exe"));
    std::cout << pid;
    if (pid == 0) { printf("error 1"); getchar(); }//error
    return 0;
}

Edit: I've changed the function to not use a std::string any more, and using lstrcmpi means it will work as either an Ansi or Unicode build. As the commentators below have suggested, you really should be using Unicode these days.