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.

Sign up for GitHub

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

Jump to bottom

Add SafeBuffer Span<T> methods #40842

Merged
merged 7 commits into from
Sep 1, 2020
Merged

Add SafeBuffer Span<T> methods #40842

merged 7 commits into from
Sep 1, 2020

Conversation

huoyaoyuan
Copy link
Member

Closes #27831

I can't find tests for SafeBuffer class, so I didn't add tests for newly added methods.

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@ghost
Copy link

ghost commented Aug 14, 2020

Tagging subscribers to this area: @eiriktsarpalis
See info in area-owners.md if you want to be subscribed.

@jkotas
Copy link
Member

jkotas commented Aug 15, 2020

SafeBuffer tests are in https://github.com/dotnet/runtime/blob/master/src/libraries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/SafeBufferTests.cs

@jkotas
Copy link
Member

jkotas commented Aug 15, 2020 •
edited
Loading

Yes, AllocHGlobal/FreeHGlobal works.

SafeBuffer construction is unsafe. The caller is responsible for passing in the correct size.

jkotas
jkotas reviewed Sep 1, 2020
View reviewed changes
...aries/System.Runtime.InteropServices/tests/System/Runtime/InteropServices/SafeBufferTests.cs Outdated
new TestStruct { I = 77, L = 88, D = 99 },
new TestStruct { I = 100, L = 200, D = 300 },
};
Span<TestStruct> structSpan = stackalloc TestStruct[]
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

jkotas
jkotas approved these changes Sep 1, 2020
View reviewed changes
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM, modulo nit about redundant test. Could you please delete it?

@huoyaoyuan
Copy link
Member Author

You mean that stackalloc-ed span is redundant with array converted one? Sure.

@jkotas
Copy link
Member

jkotas commented Sep 1, 2020

You mean that stackalloc-ed span is redundant with array converted one? Sure.

Yes, you got it. One of my comments did not show up on github for some reason.

@jkotas jkotas merged commit d4ab64e into dotnet:master Sep 1, 2020
@jkotas
Copy link
Member

jkotas commented Sep 1, 2020

@huoyaoyuan Thank you!

@vcsjones
Copy link
Member

vcsjones commented Sep 1, 2020

@huoyaoyuan Thanks a lot!

@huoyaoyuan huoyaoyuan deleted the safe-buffer-span branch September 1, 2020 20:20
@stephentoub stephentoub mentioned this pull request Sep 25, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jkotas jkotas jkotas approved these changes

Assignees
No one assigned
Labels
area-System.Collections new-api-needs-documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

API Proposal: SafeBuffer Span<T> methods
4 participants
@huoyaoyuan @Dotnet-GitSync-Bot @jkotas @vcsjones