Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Replace SafeLocalAllocHandle in System.Diagnostics.PerformanceCounter #82456

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Feb 22, 2023

Follow-up #82411

We don't need a SafeLocalAllocHandle since we own the handle and immediately call DangerousGetHandle.

Additionally change type of Interop+Kernel32+SECURITY_ATTRIBUTES.lpSecurityDescriptor from IntPtr to void* which cleans up the code.

cc: @stephentoub

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 22, 2023
@xtqqczze xtqqczze force-pushed the remove-SafeLocalAllocHandle2 branch 3 times, most recently from 53e876f to ddd8f6c Compare February 22, 2023 01:32
@xtqqczze xtqqczze force-pushed the remove-SafeLocalAllocHandle2 branch from ddd8f6c to 25c0b88 Compare February 22, 2023 21:17
@xtqqczze xtqqczze marked this pull request as ready for review February 23, 2023 04:19
@ghost
Copy link

ghost commented Feb 23, 2023

Tagging subscribers to this area: @dotnet/area-system-diagnostics-performancecounter
See info in area-owners.md if you want to be subscribed.

Issue Details

Follow-up #82411

We don't need a SafeLocalAllocHandle since we own the handle and immediately call DangerousGetHandle.

Additionally change type of Interop+Kernel32+SECURITY_ATTRIBUTES.lpSecurityDescriptor from IntPtr to void* which cleans up the code.

cc: @stephentoub

Author: xtqqczze
Assignees: -
Labels:

area-System.Diagnostics.PerformanceCounter, community-contribution

Milestone: -

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

LGTM other than two comments.

Copy link
Member

@stephentoub stephentoub left a comment

Choose a reason for hiding this comment

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

Thanks.

@stephentoub stephentoub merged commit 5f94bff into dotnet:main Mar 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-System.Diagnostics.PerformanceCounter community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants