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

SECURITY_ATTRIBUTES for windows is still not implemented #156

Closed
MaikKlein opened this issue Nov 18, 2018 · 5 comments · Fixed by #433
Closed

SECURITY_ATTRIBUTES for windows is still not implemented #156

MaikKlein opened this issue Nov 18, 2018 · 5 comments · Fixed by #433

Comments

@MaikKlein
Copy link
Member

typedef struct _SECURITY_ATTRIBUTES {
  DWORD  nLength;
  LPVOID lpSecurityDescriptor;
  BOOL   bInheritHandle;
} SECURITY_ATTRIBUTES, *PSECURITY_ATTRIBUTES, *LPSECURITY_ATTRIBUTES;

See https://github.com/MaikKlein/ash/blob/master/generator/src/lib.rs#L357-L360

Should we use a library for this? Or should we just implement it manually?

@Ralith
Copy link
Collaborator

Ralith commented Nov 18, 2018

winapi defines this. Reusing the de-facto standard crate seems nice, unless anyone can think of any serious drawbacks.

@MaikKlein MaikKlein added this to the Roadmap to 1.0 milestone Jan 19, 2019
@aloucks
Copy link
Contributor

aloucks commented May 25, 2019

Adding the winapi dependency for a single struct definition seems like overkill. I would probably just implement it manually.

@MarijnS95
Copy link
Collaborator

With windows-rs we could generate just the bindings for this type. Binding generation and compiling is crazy fast, but the Windows crate and generator itself are pretty slow to compile.

Then there's the obvious problem of crate incompatibility: crate users will have their own bindings which are type-incompatible despite carrying the same name and layout. Is there any reason this type is needed, do we have to access the fields from within Ash?

Otherwise we should probably keep all such pointers opaque types like c_void and leave it up to the caller to perform the pointer cast from their own type.

@Ralith
Copy link
Collaborator

Ralith commented May 10, 2021

I'm strongly in favor of an opaque pointer if it's sufficient. That seems to be the current case?

MarijnS95 added a commit to Traverse-Research/ash that referenced this issue May 10, 2021
`()` is an empty tuple which can be constructed, while `ffi::c_void`
cannot.  This is only ever used as an opaque pointer anyway and not used
by Ash; those wishing to access members directly should cast it to a
struct representation of choice (`winapi`, `windows-rs` or something
custom).

Fixes ash-rs#156
MarijnS95 added a commit that referenced this issue May 10, 2021
`()` is an empty tuple which can be constructed, while `ffi::c_void`
cannot.  This is only ever used as an opaque pointer anyway and not used
by Ash; those wishing to access members directly should cast it to a
struct representation of choice (`winapi`, `windows-rs` or something
custom).

Fixes #156
MaikKlein pushed a commit that referenced this issue May 10, 2021
`()` is an empty tuple which can be constructed, while `ffi::c_void`
cannot.  This is only ever used as an opaque pointer anyway and not used
by Ash; those wishing to access members directly should cast it to a
struct representation of choice (`winapi`, `windows-rs` or something
custom).

Fixes #156
@MarijnS95
Copy link
Collaborator

Otherwise we should probably keep all such pointers opaque types like c_void and leave it up to the caller to perform the pointer cast from their own type.

As noted in #950 this is not actually a pointer to easily cast, but a mutable borrow - and this doesn't allow NULL values either even though the documentation suggests it should be valid.

Perhaps we should add it to the list of "opaque types" there to solve both issues?

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

Successfully merging a pull request may close this issue.

4 participants