Support negative offsets, getting module base, and get_pid #4
Conversation
There was a problem hiding this comment.
Hey, thanks for the pull request!
As you have pointed out in some of your comments, thanks to two complement you can just cast isize to usize and end up with the same result when performing a wrapping add. If you need to have a negative offset, I'd much rather that you just do that instead of changing the offset type to isize and introducing a base address - it seems to have made the code for get_offset much more convoluted.
As I have put in review comments, I'm not sure about adding get_pid, but I do quite like ModuleInfo (although perhaps it should be renamed to something more platform-neutral like LibraryInfo?).
The equivalent on Linux seems to be dl_iterate_phdr, and if I can find and implement a similar interface for OSX I think this would be great to merge.
| /// If no process exists of that name, returns an `std::io::Error` with kind `std::io::ErrorKind::NotFound`. | ||
| /// If something went very wrong with the windows API, returns last OS error. | ||
| #[allow(clippy::cast_possible_truncation)] | ||
| pub fn get_pid(process_name: &str) -> std::io::Result<Pid> { |
There was a problem hiding this comment.
I previously hadn't added a get_pid function as there is the question of "what should you do if there is more than one process with the same name?" - which I think is something that should be left up to the library consumer instead of the library.
For example, if you are trying to look at and modify a bunch of calculator or notepad instances, then you might want to get all of the relevant PIDs or some subset, but if you're modifying a game then you only have to worry about a single process.
Do you think there'd be a good way to design a get_pid function around the possibility of multiple processes?
There was a problem hiding this comment.
I suppose I could return an iterator over all processes instead, which returns small structs with just the basics, such as process name, pid, maybe Architecture if it's detectable. Then users can .filter those as needed. Though a non-goal should be a wrapper around the entirety of winapi haha. Just, whatever is useful for messing with games, CheatEngine, etc, I suppose? Hence why having a get_pid in the first place, it's just so convenient.
I'd do a similar iterator thing for LibraryInfo.
| if tlhelp32::Module32First(snapshot, &mut module_entry) == minwindef::TRUE { | ||
| // makeshift do-while | ||
| loop { | ||
| if utf8_to_string(&module_entry.szModule) == name { |
There was a problem hiding this comment.
Aren't these UTF-16 instead of UTF-8 on Windows?
There was a problem hiding this comment.
The Microsoft docs have the type defined as CHAR. There is also a PROCESSENTRY32W equivalent, which uses WCHAR. So I assume this is... UTF-8? Or ANSI? I do not actually know, I just took that function from https://github.com/Tommoa/rs-process-memory/blob/master/examples/fastyboy.rs#L22
| return Err(std::io::Error::last_os_error()); | ||
| } | ||
|
|
||
| if tlhelp32::Module32First(snapshot, &mut module_entry) == minwindef::TRUE { |
There was a problem hiding this comment.
Why not use:
while tlhelp32::Module32Next(snapshot, &mut module_entry) == minwindef::TRUE {
...
}What does Module32Next do when you give it a null module entry?
There was a problem hiding this comment.
Because we don't want to ignore the first entry returned by Module32First. (Same goes for Process32First), and rust has no do-while loops, which is what I assume the windows devs designed those functions around?
|
|
||
| // We searched everything, nothing found | ||
| if CloseHandle(snapshot) == minwindef::FALSE { | ||
| panic!("Could not close handle") |
There was a problem hiding this comment.
I'd rather you return an error than panic.
| th32ProcessID: 0, | ||
| GlblcntUsage: 0, | ||
| ProccntUsage: 0, | ||
| modBaseAddr: std::ptr::null_mut(), // yikes |
There was a problem hiding this comment.
Null pointers. No way around it in this case, but always yikes and you won't convince me otherwise.
| @@ -1,10 +1,13 @@ | |||
| use winapi::shared::minwindef; | |||
| use winapi::um::{handleapi::CloseHandle, tlhelp32}; | |||
| use winapi::{shared::minwindef, um::handleapi::INVALID_HANDLE_VALUE}; | |||
There was a problem hiding this comment.
Change this to:
use winapi::um{handleapi::{CloseHandle, INVALID_HANDLE_VALUE}, tlhelp32};
use winapi::shared::minwindef;|
|
||
| /// Handling modules (e.g. DLLs) in a process. | ||
| pub trait ModuleInfo { | ||
| /// Gets the base address of a module in a process. For example, "GameAssembly.dll" when on Windows. |
There was a problem hiding this comment.
The equivalent for dlls on Windows on Linux and OSX are "Shared Libraries" (which usually have the extension .so). Would you mind also mentioning that in docs?
2 participants
Bit of a two-in-one PR, I can split it up if you'd prefer.
Since CheatEngine allows you to have negative offsets (and I needed that), I added support for them. Because having everything be
isizeseemed a bit iffy, there's a newbasefield inDataMemberandLocalMemberetc, which is usize, and is now what used to be the first offset.I've also added two convenience functions,
get_pid()because of course, and theModuleInfotrait to facilitate stuff such asGameAssembly.dll + 0x11223344.Sadly I only have enough experience (and motivation) to do this for Windows, sorry :(.
Hope I didn't forget anything.