Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

noticed inconsistency of generated newtype, in module Win32::Networking::Winsock: sa_family_t vs socklen_t #2627

Closed
nerditation opened this issue Aug 24, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@nerditation
Copy link

Suggestion

I was writing some code using Winsock, then I noticed the type sa_family_t is a struct with a single named field Value, while the type socklen_t is a tuple struct, which are added to the metadata at the same time:

microsoft/win32metadata#1041

also, there're other inconsistency such as sa_family_t is annotated with #[repr(C)] while socklen_t is #[repr(transparent)], sa_family_t contains a doc string about the feature flags but socklen_t doesn't; socklen_t has a #[derive(...)] attribute but sa_family_t doesn't. here's the generated rust code in the windows crate:

#[repr(C)]
#[doc = "*Required features: `\"Win32_Networking_WinSock\"`*"]
pub struct sa_family_t {
pub Value: u16,
}

#[repr(transparent)]
#[derive(::core::cmp::PartialEq, ::core::cmp::Eq)]
pub struct socklen_t(pub i32);

side note the `sa_family_t` probably should not be added, since there's `ADDRESS_FAMILY` in winsock2 already:

#[doc = "*Required features: `\"Win32_Networking_WinSock\"`*"]
#[repr(transparent)]
#[derive(::core::cmp::PartialEq, ::core::cmp::Eq)]
pub struct ADDRESS_FAMILY(pub u16);

so why is this discrepancy? does the code generator not have defined rule in this case, or is it a corner case? does this need to be fixed?

btw, is there a recommended way to convert a windows-sys type alias to the corresponding windows crate newtype? should I transmute() directly, or can I assume the newtype wrapper would always be a tuple struct (and the sa_family_t is indeed an error in code generator)? or maybe we can add a new generic api similar to core::from_abi()?

I like the fact that libraries are using windows-sys in their public api, and I also like to use the windows crate for the convenience and safety features when writing non public facing code, and often feel the need to bridge the two crate together, however, I can't find in the documentation what's the "proper" way to do the convertion.

I wonder is this a uncommon usage, or is there a well established solution and I am just missing the obiouvs? I have read #1393 and some other related issues, but I don't think they are relevant about mixing both windows-sys and windows crates.

@nerditation nerditation added the enhancement New feature or request label Aug 24, 2023
@ChrisDenton
Copy link
Collaborator

ChrisDenton commented Aug 24, 2023

sa_family_t and socklen_t were added to the metadata by request in: microsoft/win32metadata#1041

The only relevant difference between the two is that socklen_t is defined in the Windows API headers (WS2tcpip.h) whereas sa_family_t isn't. This leads to having different attributes in the metadata (NativeTypedef vs. MetadataTypedef).

I recall NativeTypedef being used to distinguish real handle types from the ones that are manually added to the metadata. I can't quite remember the details but I think that explains the difference in codegen.

In short I think this is a win32metadata issue more than a windows-rs one. Though your "btw" seems like it should be a separate issue because that does only concern windows-rs.

@nerditation
Copy link
Author

The only relevant difference between the two is that socklen_t is defined in the Windows API headers (WS2tcpip.h) whereas sa_family_t isn't. This leads to having different attributes in the metadata (NativeTypedef vs. MetadataTypedef).

ah, I didn't know sa_family_t was not in Windows API headers. it's the NativeTypedef (vs MetadataTypedef) that determines how typedefs are generated. now I understand.

I'll close this and open another one regarding the mixed usage of windows-sys and windows crates then. thank you for explaining.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants