Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Strongly type UTf8 strings using SafeHandle #337

Merged

Conversation

amaitland
Copy link
Contributor

Rename SafeUnmanagedMemoryHandle to Utf8StringHandle - it's now used in a strongly typed fashion rather than DangerousGetHandle()

The CLR will marshal a SafeHandle to and from IntPtr automatically which makes the delegates strongly typed, you can only pass in a Utf8StringHandle which is created through Utf8InteropStringConverter. Should make the API more readable.

All the InteropObjectInstance classes could be removed and the delegates could be changed to use SafeHandles, make the code more readable/maintainable in my opinion.

Let me know what you think.

…in a strongly typed fashion rather than DangerousGetHandle()

The CLR will marshal a SafeHandle to and from IntPtr automatically which makes the delegates strongly typed, you can only pass in a Utf8StringHandle which is created through Utf8InteropStringConverter
@jeremyVignelles jeremyVignelles self-requested a review October 26, 2017 07:24
Copy link
Collaborator

@jeremyVignelles jeremyVignelles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@amaitland
Copy link
Contributor Author

If .Net standard 1.3 wasn't a target I'd probably have looked at using ICustomMarshaler

https://apisof.net/catalog/System.Runtime.InteropServices.ICustomMarshaler

Something like https://github.com/nikki-nn4/NimDemo/blob/e87abf020f26e6bc98c9d0ea4faf0cdc0cd0660e/Assets/nim/Utility/UTF8StringMarshaler.cs would probably work nicely. The signatures would be long, greatly simplified usage again. Just a passing comment.

@jeremyVignelles jeremyVignelles merged commit 099a45f into ZeBobo5:develop Oct 26, 2017
@jeremyVignelles
Copy link
Collaborator

Thanks, You can check in 2.2.2-develop225 :)

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

Successfully merging this pull request may close these issues.

2 participants