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

generator: Mark QNX c_void types as opaque to get raw pointers #950

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarijnS95
Copy link
Collaborator

@MarijnS95 MarijnS95 commented Oct 3, 2024

Fixes #949

Long ago in #339 I added "opaque" types to ensure that pointers to it stay as pointers in FFI rather than get converted to &mut borrows which don't carry the load of an arbitrary pointer value to "something unknown" (= opaque). When the QNX types were added in #429 and #760 some time later, they weren't added to this list and turned into borrows making them impossible to be handled safely in user code.

Note that only SECURITY_ATTRIBUTES remains as a borrow of a c_void, which should probably be changed in some way too. If not in the least because the raw pointer is allowed to be NULL.

Long ago in #339 I added "opaque" types to ensure that pointers to it
stay as pointers in FFI rather than get converted to `&mut` borrows
which don't carry the load of an arbitrary pointer value to "something
unknown" (= opaque).  When the QNX types were added in #429 and #760
some time later, they weren't added to this list and turned into borrows
making them impossible to be handled safely in user code.
@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

making them impossible to be handled safely in user code.

They can't be handled safely regardless, I think, but at least we can have useful setters.

@MarijnS95
Copy link
Collaborator Author

@Ralith wrong wording, I think I meant UB. There are likely more safety constraints on &mut c_void (that a cast/transmute can never uphold) than there are for passing an opaque *mut c_void around?

@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

I think it's actually okay to synthesize references to ZSTs? But the lifetime isn't doing any good here.

@MarijnS95
Copy link
Collaborator Author

Makes sense. What is your verdict on SECURITY_ATTRIBUTES, before we merge this?

@Ralith
Copy link
Collaborator

Ralith commented Oct 3, 2024

That's a tricky case, since unlike a windowing handle it seems like it theoretically could address a value on the stack and readily benefit from lifetime checking. So long as we're defining it ourselves/as c_void, that's difficult but not strictly impossible for a user to take advantage of--typically going through pointer casts will erase lifetime information, but you can put it back if you really want. Do we know where they come from, and in what form?

@MarijnS95
Copy link
Collaborator Author

It's just a POD but with a pointer to a "security descriptor". Looks like the caller is expected to create it as follows:

https://learn.microsoft.com/en-us/windows/win32/secauthz/creating-a-security-descriptor-for-a-new-object-in-c--

Either way it's allowed to be NULL but that should be representable by leaving the raw/FFI pointer field at null_mut() even though our builder has no way to "unassign" it.

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

Successfully merging this pull request may close these issues.

ScreenSurfaceCreateInfoQNX builder takes c_void handles by reference instead of pointer
2 participants