newtype vs type aliases for windows-sys?

[jstarks]

In windows-sys, could it make more sense to use type aliases instead of newtypes for things like BOOL and PWSTR? It's kind of inconvenient to have to spell out the types at every callsite, especially without the conversion methods available from the full windows crate.

The precedent in other low-level C wrapper crates (winapi, libc) seems to be to expose C typedefs as type aliases, so e.g. BOOL would just be an alias for c_int or i32, etc.

[kennykerr]

Yes, I had considered that for the native typedefs. The concern is that this would make it a little less type safe, but perhaps that is less of a concern when you're using the windows-sys crate and everything is unsafe to begin with? For example, HANDLE would be isize and BOOL would be i32. It would certainly make code using windows-sys more concise.

[jstarks]

In some cases it's probably a small safety win to have the newtypes, but it may also lead to losses due to the extra transmutes or casts for pointer parameters. E.g. if a function takes *const PWSTR and I have [*const u16], then I have to do slice.as_ptr() as *const PWSTR, which seems slightly more dangerous. My intuition would be to leave them out...

The other thing I found a little dangerous is that there's no PCWSTR, so we're actually losing const correctness for the pointer types. E.g. CreateFileW takes a PCWSTR, but in windows-sys it takes a PWSTR, which is a *mut u16. Not a real safety issue since it's just a pointer, but makes it harder to use the API correctly (I have to double check in the API docs that I really can past a const pointer).

Related, it's curious that HANDLE wraps isize and not *mut c_void, making it slightly more difficult to interoperate with std's RawHandle type.

All of this would probably be addressed by staying true to the C typedefs.

[kennykerr]

I like the no-frills nature and getting closer to the original Win32 API. The metadata also provides the necessary metadata for the const variant of PWSTR - I just need to plumb it through to Rust - its on my todo list:

[MarijnS95]

@kennykerr Thanks for looking into const-correctness! That mutability cast on string literals has discomforted me more than I'd like 😁

[jstarks]

Interesting that the const attribute looks top-level but actually applies what the pointer points to. What does metadata look like for an API taking a const pointer to PCWSTR (for example, for a list of strings).

[kennykerr]

The metadata has come a long way and is pretty rich. I'd recommend grabbing ILSpy here:

https://github.com/icsharpcode/ILSpy/releases

You can then load the winmd for the win32 metadata and poke around. You can find the metadata that windows-rs uses here:

crates/deps/reader/default

[kennykerr]

@jstarks I took that for a spin and I do like the results. The resulting code looks and feels a lot more like what it would in C/C++:

https://github.com/microsoft/windows-samples-rs/compare/sys-handles?expand=1

[jstarks]

Cool. What do you think about additionally changing HANDLE to *mut c_void instead of isize? That will give back a little safety for handles (can't accidentally pass in some random integer) while also aligning with std::os::windows::io::RawHandle.

[kennykerr]

The Win32 metadata defines it as IntPtr rather than void* which means windows-rs maps it to isize rather than *mut c_void. If you think that's "more correct" then I'd prefer to push for the change in the metadata and then windows-rs will automatically map it to the appropriate underlying type. You can open an issue here:

https://github.com/microsoft/win32metadata/issues

[jstarks]

Looks like there was some discussion several months ago: microsoft/win32metadata#21. I haven't had time to digest this yet, but it looks like perhaps it was changed based on feedback from .NET.

[kennykerr]

OK, let's figure out what that's about and go from there. We can always override that for Rust but I'd like to have a better understanding first.

[kennykerr]

Hmm, looks like Rust really really wants this to be a pointer:

https://doc.rust-lang.org/std/os/windows/raw/type.HANDLE.html

I may just override this for Rust and move on... It sure does make porting system code easier.

[kennykerr]

And this is now so: #1332

[kennykerr]

#1550 adds first-class types for PCSTR and PCWSTR.