Windows unicode (_W) api for all Win32 functions
#89
Conversation
|
I believe this would be a good change, since using the wide function are generally recommended and would make their usage consistent throughout the codebase. Just needs to be rebased. |
Crzyrndm
force-pushed
the
windows-unicode-api
branch
from
76a4156
to
82cb202
Compare
This has been (squashed and) rebased. I lack the hardware to test this that I had last year though :/ |
|
Thank you very much for your PR @Crzyrndm and for the recent attention @RossSmyth!
Did you had a piece of hardware returning actual UTF-16 codepoints? |
|
I recommended this change since we saw issues with reporting device serial numbers as fixed by: #63 It's kind of a case of "if you can do, then someone will do it." But I doubt it would be a common issue. |
There was a problem hiding this comment.
Looking over the changes there were several points which caught my attention.
The API provided by the windows crate does't seem to bring more ergonomics witch respect to this buffer handling here. But couldn't using u8 views bring an some ergonomics at least in one direction? Or may be there is a crate out for dealing with this somewhat challenging buffer API.
|
If desired I can give this a more thorough review later in the week.
If it is desired to move to the Windows crate I would recommend that windows-sys is used and not windows-rs wrt to buffer handling, the wide literal macro could be copied for now to try and reduce allocation if that is important. |
individual unsafe blocks driveby resolve clippy lint add `Modem` as a device class to query for COM ports some refactoring of `get_ports_guids` to handle multiple class names clarifying comment filter to COM ports rather than removing LPT (unknown what else modems could show up as) raw ports only need to be iterated if additional ports have been found resolve typo in comment str -> w_string function critique the moethod later... `get_ports_guids` using `_W` win32 API `PortDevices::new()` using `_W` win32 API `PortDevices::instance_id()` using `_W` win32 API `PortDevices::name()` using `as_utf16` add utf16->utf8 function handles the conversion and null trimming use `from_utf16_lossy_trimmed` `PortDevice::property()` handling a larger than expected property by retrying `get_registry_com_ports` using utf16 API also removed all the unused optional parameters from `RegQueryInfoKey` minor cleanup
Let's use the same type names, variable names, and the same checks for easily comparing them.
There is neither an explanation in the code nor does the history of this check reveal any insight in why it should be present. If the buffer is too small, what are we getting then, truncated data? I don't see the point in processing this data.
Let's check it the same way as for other registry data.
Harmonize the buffer sizes used here. If we are already in the mood for spending 100 characters, MAX_PATH won't hurt either.
sirhcel
force-pushed
the
windows-unicode-api
branch
from
e932ae4
to
bb2a95a
Compare
|
Rebased to resolve conflict with main branch. |
|
Thank you for pushing forward with this PR @Crzyrndm! See my replies to #89 (review). Resolved the merge conflict with main and added some more cleanup. A local test with
This would be great @RossSmyth. |
Co-authored-by: Christopher Smyth <[email protected]>
Based on #84.
The new changes
Originally posted by @mlsvrts in #84 (comment)
Originally posted by @mlsvrts in #84 (comment)