FLHook SVN discussion
-
Well sizeof can be used for char, for wchar its better to use _countof. wsclen is just the length of the string but not the size of the buffer that holds these strings
And this is not only for Cannons plugin, that is also in the base FlHook version and in some other plugins too
-
I am not sure I follow you. The base FLHook does not even use the overloaded call for swprintf where you also pass the buffer length. There are only a few calls which use sizeof and I agree that they are problematic.
-
Schmack, I’ve added you to the dev group. You can now patch things yourself.
Huor, you don’t have a Forge account just yet, but if you do make one I can add you too.
-
It’s in SVN, DJ.DBFuz: http://svn.the-starport.net/flhookplugin/
-
@FF Thanks, I will correct it when I find the time. Maybe I add some stuff I recently reversed, too.
-
FriendlyFire wrote:
…Huor, you don’t have a Forge account just yet, but if you do make one I can add you too.Now i remember i already have an account - dunno since months, but it does not let me in. Yesterday i tried again but it refuses - even with a new password. So there is something wrong. Maybe someone might have a look into this?!
Thanks in advance.
-
I’ve checked and I do not see a user named “huor” so I think you don’t have an account.
-
Just to come back to topic. Its something i am currently stumbled.
In some of the function calls FlHook uses a variable argument list to handle multiple arguments. However such a variable argument list must be started withva_start ()
call. So far so good, but to clean the stack it is required to have the
va_end ()
as well, which i searched for, is missing in every call. I am not sure what happens if its left, but i learned that both should be used together. Whether i am not sure if thats important - i corrected it now in my version.
-
Well, if you look at the documentation for the msvc compiler there only is stated that the pointer is set to NULL. Nowhere is explicitly written that you have to call it. The example with void testit(…) doesn’t. The one after that does.
If you look on www.cplusplus.com they do state this. Whether this causes errors - no idea. Doesn’t seem to, though. With gcc etc this might be different.
-
Just want to mention that the last added revision from w0dka (236) seems to miss the pushad and popad commands within the asm code [in dynamic_missions/Hooks.cpp]
Schmack already pointed that out in the base plugin code where it was corrected already. Just thought to mention it.
-
Yeah, I see what you mean. I’d call it risky, since the registers after the calls from the asm code could contain different values than the msvc compiler assumes while compiling, leading to unexpected behaviour.
-
Any function you call will clean up and preserve registers itself. A function that does not do that is generally buggy/faulty.
Thats why you dont need pushads/pops around “call xy”.As regards __asm blocks, msdn has this to say:
When using __asm to write assembly language in C/C++ functions, you don’t need to preserve the EAX, EBX, ECX, EDX, ESI, or EDI registers.
You should preserve other registers you use (such as DS, SS, SP, BP, and flags registers) for the scope of the __asm block. You should preserve the ESP and EBP registers unless you have some reason to change them (to switch stacks, for example).
-
Yes. this sounds all nice, however since there also is stated:
However, using these registers will affect code quality because the register allocator cannot use them to store values across __asm blocks. In addition, by using EBX, ESI or EDI in inline assembly code, you force the compiler to save and restore those registers in the function prologue and epilogue.
The compiler seems to analyse the inline asm code, which ofc does not work when doing external calls. For example the second function uses edi, but the compiler never saves it before and after the asm block (i’ve checked with IDA), but reads it later.
Notice, that I still am not quite sure how to interpret the text exactly. I’ve checked the generated assembly code of such blocks some time ago and came to the conclusion that it is not quite clear how save it is, since it is very time consuming to check all the cases.
