stevemk14ebr / PolyHook_2_0

C++20, x86/x64 Hooking Libary v2.0
MIT License
1.58k stars 222 forks source link

Design issues with bool hook() and bool unHook() #182

Closed BullyWiiPlaza closed 1 year ago

BullyWiiPlaza commented 1 year ago

Hi,

when calling the hook() and unHook() methods, the return value is bool to indicate success or failure. Generally, it would be great to also get error details in the fashion of GetLastError() but object-oriented. Simply returning false on every different error case is not helpful enough to figure out what really went wrong so debugging the code or reading the log is no longer the go-to approach.

However, since we're already on modern C++, I'd personally prefer if the API was migrated towards throwing (custom) exceptions with helpful error messages, for example like this:

if (!disassembled_successfully) {
    throw std::runtime_error("Disassembling memory failed");
}

In that case, you'd make the API void hook() and void unHook() respectively since the return value will no longer be needed when using exceptions.

Now, maybe not everyone likes to use exceptions or some systems may not even support exceptions properly. Therefore, you can offer both if you implement something like the std::filesystem API has done.

stevemk14ebr commented 1 year ago

This exists, get an instance of the errorlog class then invoke pop to see each error that occured internally when either call returns false. https://github.com/stevemk14ebr/PolyHook_2_0/blob/b0aac632f427cd730c88b9b5198c6669aa05d02f/sources/ErrorLog.cpp#L44

Exceptions are quite bad, and not suitable for embedded systems, I will not implement them. A better error passing system is how rust does it with result/optional types, but this is not a good interface for interop-ing with raw C which is a design goal at the api boundary.