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

Merge pinvoke guidelines into interop guidelines #34833

Merged
merged 2 commits into from
Apr 10, 2020

Conversation

danmoseley
Copy link
Member

Fixes #304

I separated out some edits and the merge.


If you *do* use `StringBuilder` one last gotcha is that the capacity does **not** include a hidden null which is always accounted for in interop. It is pretty common for people to get this wrong as most APIs want the size of the buffer *including* the null. This can result in wasted/unnecessary allocations.

**[USE]** Char arrays from `ArrayPool` or `StringBuffer` or `stackalloc`
Copy link
Member Author

Choose a reason for hiding this comment

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

I added stackalloc here. Probably, we should explicitly talk about buffer allocation in general (not just char buffers)


Interop signatures / structs / constants should be defined using the same name / capitalization / etc. that's used in the corresponding native code. We should not rename any of these based on managed coding guidelines. The only exception to this is for the constant grouping type, which should be named with the most discoverable name possible; if that name is a concept (e.g. Errors), it can be named using managed naming guidelines.
Copy link
Member Author

Choose a reason for hiding this comment

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

I left this bit in as the doc only recommends constants be named this way.

Copy link
Member Author

Choose a reason for hiding this comment

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

BTW that topic probably should mention stackalloc given how often we use it for interop buffers. And suggest a max size.

Copy link
Member

Choose a reason for hiding this comment

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

The doc has the recommendation for everything. It just has the text about the constant grouping type replaced with "CONSIDER" recommendation (vs. DO recommendation for method names) .

DO use the same naming and capitalization for your methods and parameters as the native method you want to call.
CONSIDER using the same naming and capitalization for constant values.

Copy link
Member

Choose a reason for hiding this comment

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

There is a whole chapter on the constant grouping types before this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right - we were saying "should" vs the doc just says CONSIDER. I am happy to remove it though.

Copy link
Member Author

Choose a reason for hiding this comment

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

removed it

@jkotas jkotas merged commit ed6f88a into dotnet:master Apr 10, 2020
@danmoseley danmoseley deleted the pinvoke.md branch April 10, 2020 23:47
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
@danmoseley danmoseley restored the pinvoke.md branch December 22, 2020 05:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

organize markdown files
4 participants