getProcID and getProcessHandle functions

Question

I made two functions: one to get process id and the other to get a handle with all access to that process using the process id. I'm looking for help with making these easy to use on all types of projects. The only thing I know is maybe letting the person chose the open process properties.

DWORD getProcID(const std::wstring &windowName, std::string &status)
{
    DWORD processID = NULL;
    HWND windowHandle = NULL;

    windowHandle = FindWindowW(NULL, windowName.c_str()); // Gets handle to top window (class name, window name)
    if (windowHandle) {
    GetWindowThreadProcessId(windowHandle, &processID); // returns thread that made window (window, process for window)
            if (processID != 0) {
                    return processID;
            }
            else { // if process id is bad 
                status = "Error with process";
                return 0;
            }
    }
    else { // if window name is bad/not there
        status = "Error with window";
        return 0;
    }
    CloseHandle(windowHandle); // close window handle since we dont need it anymore
    return 0;
}

HANDLE &getProcessHandle(const DWORD processID, std::string &status) {
    HANDLE procHandle = NULL;

    if (processID != 0) {
        procHandle = OpenProcess(PROCESS_ALL_ACCESS, FALSE, processID); // returns handle to process (what you want, inherit stuff, process id)
        if (procHandle != INVALID_HANDLE_VALUE || procHandle != NULL) { // if handle returned by openProcess is good
            status = "Game is ready to hack";
        }
        else { // if handle retured by openProcess is bad
            status = "Error with handle";
        }
    }
    else {
        status = "Error with process id";
    }
    return procHandle;
}

Answer 1

I fairly routinely (not so much here as at work, for example) kind of "scold" people for creating classes where they really only need or want a single function.

This, however, is kind of the opposite case: functions where you really want classes. In particular, RAII can provide a real benefit here, and for that you need a class with a dtor.

I'm not excited about the situation with reporting errors by putting them into a string (and hoping that an empty string indicates success). I'd rather follow the model of "succeed or throw".

To keep that simple, I think it can also benefit from defining an assure (or something on that order) that's at least roughly similar to assert, except that it remains intact in release builds, and (perhaps) throws an exception instead of just aborting if a problem is seen. For example:

#define assure(cond, msg) do { if (!(cond)) throw std::runtime_error(msg); } while (0)

With that in hand, I'd probably start with a WindowHandle type something like this:

class WindowHandle {
    HWND handle;
public:
    WindowHandle(std::wstring const &windowName) 
        : handle(FindWindowW(nullptr, windowName.c_str())) 
    { 
        assure(handle != 0, "Window not found");
    }
    operator HWND() { return handle; }
};

Note that (at least as far as I know, or MSDN seems to indicate) you normally don't want to call CloseHandle on the handle returned from FindWindow. If you did want to, you'd do that in the destructor.

Likewise, a process ID would look something like this:

class ProcId {
    WindowHandle handle;
    DWORD id;
public:
    ProcId(std::wstring const &window_name) 
        : handle(window_name)
    {
        GetWindowThreadProcessId(handle, &id);
        assure(id != 0, "Error retrieving process ID");
    }
    operator DWORD() { return id; }
};

Finally, we get to the process handle (where we finally do need to use Closehandle):

class procHandle {
    ProcId id;
    HANDLE handle;
public:
    procHandle(std::wstring const &name, DWORD access = PROCESS_ALL_ACCESS)
        : id(name), 
        handle(OpenProcess(access, FALSE, id)   )
    {
        // according to MSDN, returns NULL in case of error
        assure(handle != NULL, "Error retrieving process handle");
    }

    ~procHandle() { CloseHandle(handle); }

    operator HANDLE() { return handle; }
};

Note how each of these builds on the previous, so each only does one thing (i.e., we're following the single responsibility principle). Also note how the code to open a process handle becomes completely trivial:

procHandle p(L"SomeWindowName");

If we add in code to print out the error message if something goes wrong, it's still only marginally more complex:

try {
    procHandle p(L"Start");
    // Code to use process handle goes here.
} // handle will be closed automatically when we exit the scope
catch (std::exception const &e) {
    std::cerr << e.what() << "\n";
}

At least to me, this seems both easier to understand, and easier to use (and harder to use incorrectly).