Skip to content

Provide an additional friendly overload with raw unconvertible handles when necessary#1693

Open
DoctorKrolic wants to merge 1 commit into
microsoft:mainfrom
DoctorKrolic:promote-unconvertible-handles
Open

Provide an additional friendly overload with raw unconvertible handles when necessary#1693
DoctorKrolic wants to merge 1 commit into
microsoft:mainfrom
DoctorKrolic:promote-unconvertible-handles

Conversation

@DoctorKrolic
Copy link
Copy Markdown
Contributor

Previously if friendly overload accepted a handle with a known release method CsWin32 would always "promote" it to a SafeHandle. This works well if CsWin32 can trivially represent that handle as a SafeHandle so that function that opens/creates the handle in the first place returns it a a custom generated SafeHandle. However, if release method is a complicated one and CsWin32 cannot generate a trivial SafeHandle wrapper based on it user was forced to declare such safe handle type on their own to use it in other friendly overloads. This might not be ideal e.g. if the handle is used whithin one method frame so that declaring a safe handle type for it is an overkill. Now CsWin32 generates another overload with such handles represented as "raw" handles so that user has a choice either to use the raw handle or to declare their own safe handle type and use it instead

@jevansaks
Copy link
Copy Markdown
Member

There is already a safeHandles knob in NativeMethods.json. How is this different than letting the consumer choose a different form of the projection? More overloads can just be more confusing.

It sounds like from the description this is limited to "complex" handles, could you give some concrete examples for those and how this would help?

@DoctorKrolic
Copy link
Copy Markdown
Contributor Author

There is already a safeHandles knob in NativeMethods.json. How is this different than letting the consumer choose a different form of the projection?

useSafeHandles switch disables all safe handles, while this change targets a very specific use case. Some handles aren't trivially closable, i.e. their annotated close method takes 2 or more parameters. In such cases CsWin32 doesn't generate a safe hanlde implementation for them, so the function openning the handle just returns a raw handle instance. At the other hand functions which accept the handle still require a SafeHandle for that handle parameter. This means that a user must declare their own safe handle wrapper to be able to use friendly overloads

could you give some concrete examples for those and how this would help

The added test functions show this quite well. In order to enumerate WFP filters you need to open an enum handle via FwpmFilterCreateEnumHandle0. Since the returned handle is closed via FwpmFilterDestroyEnumHandle0, which takes 2 paramneters (session handle + enum handle), CsWin32 cannot generate a safe handle wrapper for it and returns it as out FWPM_FILTER_ENUM_HANDLE. To actually enumerate filters function FwpmFilterEnum0 is used. It takes enum handle as its second parameter. Since it is a closable handle CsWin32 promotes it to a SafeHandle. So now as a user I get the handle as FWPM_FILTER_ENUM_HANDLE, but need a SafeHandle on a use site, so I have to declare a safe handle wrapper myself. This might be an overkill. In my actuall use case I needed to enumerate filters until I find a permanent one on a given sublayer, so declaring my own safe handle just to wrap the value in it and dispose at the end of the method felt like unnecessary boilerplate. With this change 2 sets of overloads will be generated for such functions: in one all handles stay SafeHandles and in the second one such 'unconvertable' handles are downgraded to the 'raw' handle types. This gives a user flexibility to either declare a safe handle and use it if the handle is long lived and automatic lifetime tracking is desired or just simply use the raw handle if their logic is simple enough

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.

2 participants