feat: _winapi.GetModuleFileName#4098
Conversation
| #[cfg(windows)] | ||
| { | ||
| "win32api" => win32api::make_module, | ||
| } |
There was a problem hiding this comment.
_winapi.GetModuleFileName is part of _winapi module. We don't need to add our original module for the purpose.
check winapi.rs file for the module.
|
You might be looking for "feat". "feta" is a kind of cheese. |
@fanninpm you have permission to edit any title or text |
| } | ||
| } | ||
|
|
||
| const MAX_PATH: usize = 260; |
There was a problem hiding this comment.
@youknowone Do you have any idea to set MAX_PATH?
There was a problem hiding this comment.
I am not an expert of windows API, but reusing it from windows-rs?
https://microsoft.github.io/windows-docs-rs/doc/windows/Win32/Foundation/constant.MAX_PATH.html
I found another article about it: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=registry
There was a problem hiding this comment.
https://github.com/python/cpython/blob/v3.10.5/Modules/_winapi.c#L1309-L1324
I found that in the implementation of cpython, it uses WCHAR[MAX_PATH](WCHAR = ctypes.c_wchar
) and ctypes.c_wchar has a fixed size of 2 bytes nvaccess/nvda#8981.
So it's fine to reuse MAX_PATH of windows-rs.
youknowone
left a comment
There was a problem hiding this comment.
In general, looks very good. I left a few comments.
I will try to test this one with the http update because we don't have test for this function.
| #[pyfunction] | ||
| fn LoadLibrary(path: PyStrRef, vm: &VirtualMachine) -> PyResult<isize> { | ||
| let path = path.as_str().to_wide_null(); | ||
| let handle = unsafe { LoadLibraryW(PCWSTR::from_raw(path.as_ptr())).unwrap() }; | ||
| if handle.is_invalid() { | ||
| return Err(vm.new_runtime_error("LoadLibrary failed".to_owned())); | ||
| } | ||
| Ok(handle.0) | ||
| } |
There was a problem hiding this comment.
This function seems not a _winapi function. By searching, ctypes? but we don't have one.
I need to think where to put this one.
There was a problem hiding this comment.
Sorry, I forget to mention that it's for my personal test only.
Before getting a module's file name, I must load someone first.
| use std::ffi::{OsStr, OsString}; | ||
| use std::os::windows::prelude::*; |
There was a problem hiding this comment.
we prefer to keep imports in a single group. relocate to line 23 will be the best (and cargo fmt --all to sort again)
8657469 to
ebafe66
Compare
youknowone
left a comment
There was a problem hiding this comment.
I merged it with the HTTP tests to check if it runs as expected.
Thank you for contributing!
close #4001
close #3925 by including its all commits