Several improvments possible for MH_RemoveHook

[Elmue]

Your code is very inefficient if multiple hooks are to be removed at once.
I'am removing 16 hooks with MH_RemoveHook() and each time all threads are enumerated and each time all threads are frozen.
This is very slow.

The function MH_RemoveHook should be replaced by:

MH_RemoveHooks(LPVOID pTargetArray, UINT uArrayCount)
{
     Enumerate all threads,
     Freeze all threads,
     Delete uArrayCount hook entries,
     Unfreeze all threads
}

---

Another very useful improvement would be that MH_RemoveHooks takes the Detour functions instead of the Target functions.

Why ?
Because I store all detour functions in variables in my class which I need to call the original function.
But the target functions are not used anymore after I have set the hook.
So if I want to later remove hooks I need 32 variables!

I need 16 variables for the detours that I call inside the hook.
And I need 16 additional variables to store the targets which I need only to later remove the hooks.

So the perfect function would be:

MH_RemoveHooks(LPVOID pDetourArray, UINT uArrayCount)
{
     .....
}

Then I don't need to store the target function anymore.

---

And the absolute perfect MH_RemoveHooks function would allow me to pass a NULL pointer for pDetourArray which will remove ALL existing hooks.

[m417z]

You can disable all hooks in one go before removing them to have the thread enumeration run only once:

MH_DisableHook(MH_ALL_HOOKS);

Alternatively, if you need to disable/remove only some of the hooks efficiently, you can use MH_QueueEnableHook/MH_QueueDisableHook, then MH_ApplyQueued.

[Elmue]

I know.
But I do not want to disable them.
I want to remove them because I'am hooking a function in a DLL that has been unloaded by the process.
So the memory that these hooks and their trampolines are occupying should be freed.
This application loads and unloads the DLL several times that I'am hooking.
This is stupidly programmed, but it is not my application.
I'am hooking a DLL in a third party application and it would be useful to REMOVE all hooks with one command.
Currently this is very clumsy.

[m417z]

You can also just use MH_Uninitialize which suspends the threads once and removes all hooks.

If you want to remove all hooks without uninitializing the library, currently the best way is disabling them like I mentioned in the previous message, and then removing them one by one. Removing a disabled hook doesn't suspend all threads.

Generally, it's unclear if you're asking about performance or convenience. Your first message starts with "Your code is very inefficient", and your second message ends with "Currently this is very clumsy".

[Elmue]

It is both: A performance and a convenience issue.
If you have 70 hooks and want to remove 50 of them and remove them one by one and each time all threads are enumerated and frozen, then this will take an entire second. So this is a performance issue.

When I have to write a function that first disables all hooks and then removes them one by one, my question is:
Why do I have to write this function? Why is it not already part of the library for convenience?

MH_DisableHook(MH_ALL_HOOKS);

This is no solution for me because I have hooks set on NtDll.dll that must always stay active.
And I have other hooks on another user DLL that must be removed when the DLL is unloaded.
Disabling all hooks is not a solution.

My proposal stays the best solution.
Passing an array with all hooks to be removed.
The first for( ) loop checks if a hook is enabled.
If at least one hook is enabled, suspend the threads.
In a second for( ) loop remove the hooks.
Then start the threads again.

These are only a few lines of code and it would be useful for performance and for convenience to have such a function.

---

Additionally the code is completely free of comments.
It should be documented that calling MH_RemoveHook() multiple times may be slow.

Removing a disabled hook doesn't suspend all threads.

Also this information should be explained in the file MinHook.h
If you do not write a detailed documentation, how will people use this library efficiently?

Why are all programmers on Github so stingy with comments ??

[Nucleoprotein]

I think it can be good to add function to remove all disabled hook 🤔
Like: use MH_QueueDisableHook then MH_ApplyQueued to disable that 50 hooks then new function like MH_RemoveDisabled() to remove them? 🤔 or like MH_RemoveHook(MH_REMOVE_DISABLED) 🤔

[m417z]

MH_RemoveDisabled()

I actually have something like this in my fork, where I feel more comfortable making major changes or experimenting:

https://github.com/m417z/minhook/blob/fdcbfb2253604c3e96bc6848bca4cc00796aafc4/include/MinHook.h#L189

Here I mainly fix straightforward and/or critical issues.

[Elmue]

@Nucleoprotein

I think it can be good to add function to remove all disabled hook 🤔
Like: use MH_QueueDisableHook then MH_ApplyQueued to disable that 50 hooks then new function like
MH_RemoveDisabled() to remove them?

This is a good proposal.

And if you add this, don't forget to write a comment in MinHook.h that explains how to remove multiple hooks without performance issues.

---

Also it would be useful to allow to pass the detour pointer instead of the target pointer.
The target pointer is not used nowhere in my code.
I have to store it only for later removal.
So it would be very practical if I would not need to have several variables where I have to store the target pointers.

The function FindHookEntry() could be expanded to:
FindHookEntry(BOOL bDetour, LPVOID pFuncPointer)
where
FindHookEntry(TRUE, pFuncDetour) searches by Detour function and
FindHookEntry(FALSE, pFuncTarget) searches by Target function.

Then the same applied to the other functions would result in
QueueHook(BOOL bDetour, LPVOID pFuncPtr, BOOL queueEnable)
MH_RemoveHook(BOOL bDetour, LPVOID pFuncPtr)
etc