Conversation
SourceHook and CDetour with KHook`SourceHook and CDetour with KHook
|
Are we still concerned about using STL on public interfaces? things will get weird with gcc vs clang extensions. |
That's a good point. I've mainly stripped every instances of the word That said, looking at the two interfaces affected Core & bridge are built as a package deal so we probably don't have to worry about diverging STL implems. Unless we want to preserve that flexibility in hot swapping core and bridge libs, in which case I can just bring |
48b5f88 to
56f811c
Compare
5ebea47 to
d0d342a
Compare
ef3a114 to
ff8cc50
Compare
9e63d38 to
871a86b
Compare
* Fix dhooks float arguments on x64 * Fix inverted check in DynamicDetour.Disable * Fix re-entrant callback removal on plugin unload * Adapt sdkhooks/sdktools/dhooks to KHook * Fix SendFile hook signature on L4D2 engine * Fix dhooks corrupting bool object pointers * Fix sdkhooks vtable hooks leaking on bulk unhook * Fix dhooks return value leak in hook callbacks * Fix later dhooks callbacks overriding a supersede * Save the first float argument register on x64 * Zero-initialize dhooks register save arrays * Defer sdkhooks vtable hook deletion out of hook callbacks * Fix dhooks thiscall argument count * Allow nulling a dhooks entity return value with -1 * Remove leftover dhooks debug class and printf * Fix dhooks dynamic hook ownership and teardown leaks * Fix dhooks dynamic hook teardown leak * Remove unused JIT page protection call in dhooks * Different approach to deferred vtable deletion * Fix sdkhooks always blocking EndTouch * Fix DHookGetReturnString null check * Fix sdkhooks overriding OnTakeDamage return * Additional hardening on hook teardown just to be safe
* Fix use-after-free when removing vtable hooks * Deduplicate sdkhooks hook dispatch code * Keep hook deletion async * Erase hooks if no callbacks are left * Edit diff history --------- Co-authored-by: Kenzzer <14257866+kenzzer@users.noreply.github.com>
d2ec40b to
274f4f9
Compare
Headline
left a comment
There was a problem hiding this comment.
After more carefully reading through the changes to MM:S I skimmed this SM side of resulting changes (and i skipped dhooks changes entirely) - I feel this slotted in pretty nicely all things considered. Any comments I felt I could make are trivial overall.
For such a massive change it's obviously difficult to review, but it's clearly matured and is obviously the next step
| { | ||
| if (m_Deps.find(*pInfo) == m_Deps.end()) | ||
| auto iter = m_Deps.begin(); | ||
| while (iter != m_Deps.end() && !((*iter) == (*pInfo))) |
There was a problem hiding this comment.
why not std::find for this and m_Dependents
There was a problem hiding this comment.
Going to be completely honest, I was not thinking when doing these changes. Just whatever got me to the finish line of stripping SH containers haha. Anyways yeah thanks, I'll apply the suggested change
| flags |= PASSFLAG_OCTOR; | ||
|
|
||
| //if (pCopyCtor) | ||
| // flags |= PassInfo::PassFlag_CCtor; |
There was a problem hiding this comment.
As far as I can tell this was not needed even with SH since all call sites passed NULL. Maybe this is a good opportunity to clean it up though, since the other 3 params are effectively dead too. Callers set these passflags by hand on the PassInfo. Could slim down the signatures a bit:
void SetReturnType(size_t size, PassType type, int flags);
void AddParam(size_t size, PassType type, int flags);There was a problem hiding this comment.
This got answered, but yes. When I was making those changes to bintools, I realised a few statements were in reality unnecessary.
But I did not do any cleanups afterwards.
In the continuity of alliedmodders/metamod-source#223. This PR strips
SourceHookandCDetourfrom SourceMod.Marked as a Draft until I finish writing the x86 assembly for dhooks on windows, and I'd like to have some unofficial builds tested around first. Local testing looks good though!
Breaking changes
Benefits
Those breaking changes are definitively not ideal, and I went in working on this knowing full well the PR might die at the finish line.
In the PR's defense, I have worked on this x86_64 support project (MM:S, SM, KHook) on and off for nearly 2 years, and in that timespan nobody has offered something close to a working alternative. I believe this is as good as it will ever get..