Strong typed Wh_SetFunctionHook

[learn-more]

For my own code I like to use a strong-typed Wh_SetFunctionHook wrapper:

template<typename ProtoType>
BOOL Wh_SetFunctionHookT(ProtoType targetFunction, ProtoType hookFunction, ProtoType* originalFunction)
{
    return Wh_SetFunctionHook((void*)targetFunction, (void*)hookFunction, (void**)originalFunction);
}

When used with decltype and winapi functions, this allows to detect prototype mismatches:

This will compile just fine:

using FindWindowW_t = decltype(&FindWindowW);
FindWindowW_t FindWindowW_Original;
HWND WINAPI FindWindowW_Hook(LPCWSTR lpClassName, LPCWSTR lpWindowName)
...
Wh_SetFunctionHookT(FindWindowW, FindWindowW_Hook, &FindWindowW_Original);

This won't compile (note the mismatched return type):

using FindWindowW_t = decltype(&FindWindowW);
FindWindowW_t FindWindowW_Original;
BOOL WINAPI FindWindowW_Hook(LPCWSTR lpClassName, LPCWSTR lpWindowName)
...
Wh_SetFunctionHookT(FindWindowW, FindWindowW_Hook, &FindWindowW_Original);

[m417z]

Nice. In Windhawk v1.3, I added the windhawk_utils.h header file with a couple of C++ helper functions. Wh_SetFunctionHookT can be a nice addition. I'm moving the issue to the windhawk repo.

[learn-more]

Do you have an idea for an upgrade path?

My recent mod has this function inline defined, but it would be a bit of an issue if the mod only works on the old or the new version:
https://github.com/ramensoftware/windhawk-mods/blob/da7734260f30db5e6b11898551f6a027f0df4b03/mods/lm-regedit-multi-instance.wh.cpp#L36

Or is there a mod option that tells which Windhawk version it minimally needs?

One idea would be that there is a define that can be checked, and if that's set I can include windhawk_utils.h and skip this inline definition.

[learn-more]

Depending on how readable the error is, it might make sense to add a comment or something near that prototype explaining that compile errors on it mean that one of the prototypes is wrong btw. (Considering that Windhawk might also be used by people that are not a developer by profession ;) )

[m417z]

Do you have an idea for an upgrade path?

I'm not sure what you mean by an upgrade path, but here's how it currently impacts users and developers:

Users: Trying to install an incompatible mod will result in a "Compilation failed" message. That's not optimal, as there's no indication for the error reason and a user can't know that updating Windhawk will solve it. An average user might not even know what compilation means. Newer Windhawk versions show a more useful message similar to "Compilation failed, you might need to update Windhawk".

Developers: The situation here isn't optimal either. For a developer using the latest version of Windhawk, it's difficult to know how backwards compatible the code is, and checking it is tedious. For my mods, if I see that I use newer features, I try to add the required Windhawk version in the readme or near the relevant setting, but it's easy to miss. That's the main reason I didn't document windhawk_utils.h yet.

P.S. Even your simple regedit mod requires Windhawk v1.2 due to the usage of the WH_MOD_ID, WH_MOD_VERSION definitions. That's totally my fault, as I added them to the new mod template and didn't think of the backwards compatibility implications.

There are various things that can be done to improve this, like running GitHub actions to verify compatibility and adding a minimal Windhawk version to metadata, which I might look at in the future.

One idea would be that there is a define that can be checked, and if that's set I can include windhawk_utils.h and skip this inline definition.

I did something similar with the volume control mod:
https://github.com/ramensoftware/windhawk-mods/blob/da7734260f30db5e6b11898551f6a027f0df4b03/mods/taskbar-volume-control.wh.cpp#L65-L68

In that case, the header is only needed for a secondary feature. So the mod is compatible with older Windhawk versions, but this feature won't work. Even worse, it won't work even after Windhawk is updated unless the mod is recompiled. So now, in hindsight, I don't think that it was a good idea.

But you're talking about a case in which the main mod functionality requires the header. In this case, why would you conditionally include the header and conditionally inline? You can just inline the implementation and support all Windhawk versions. Many of my mods have a function to load and cache symbols, it's about 200 lines long and it's ugly to copy paste it every time, but I do it for backwards compatibility. That was the main motivation for windhawk_utils.h, and I plan on using it more as time goes by and more users update to a newer Windhawk version.

Depending on how readable the error is, it might make sense to add a comment or something near that prototype explaining that compile errors on it mean that one of the prototypes is wrong btw. (Considering that Windhawk might also be used by people that are not a developer by profession ;) )

That's a good idea, perhaps a static_assert. I'll check it out when I get to it.

[m417z]

I'm working on a new Windhawk version and looking into including Wh_SetFunctionHookT. After trying several variants, I got to:

template <typename Prototype>
BOOL Wh_SetFunctionHookT(Prototype* targetFunction,
                         Prototype* hookFunction,
                         Prototype** originalFunction) {
    return Wh_SetFunctionHook(reinterpret_cast<void*>(targetFunction),
                              reinterpret_cast<void*>(hookFunction),
                              reinterpret_cast<void**>(originalFunction));
}

It's very similar to your variant but imposes slightly more constraints.

That's a good idea, perhaps a static_assert. I'll check it out when I get to it.

I tried improving the error messages. One option I experimented with is this:

template <typename P1, typename P2, typename P3>
BOOL Wh_SetFunctionHookT(P1 targetFunction,
                         P2 hookFunction,
                         P3* originalFunction) {
    static_assert(std::is_same<P1, P2>::value, "Hook function prototype doesn't match the target function");
    static_assert(std::is_same<P1, P3>::value, "Original function prototype doesn't match the target function");
    return Wh_SetFunctionHook((void*)targetFunction, (void*)hookFunction,
                              (void**)originalFunction);
}

The error is noisy but slightly better than the first version. The downside is that the IDE doesn't mark the call site with an error, which is only detected when compiling, so I think it's not worth it.

I also tried concepts, it works but there's no way to use a custom message so it's not really better than the first version.

If you have any other ideas, let me know. Otherwise I'll go with my first version.

[learn-more]

Seems like a nice solution indeed!

[m417z]

Version 1.4 includes Wh_SetFunctionHookT in windhawk_utils.h.