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

Implement Marshal.Get(Last)PInvokeErrorMessage() APIs. #70685

Merged
merged 7 commits into from
Jun 24, 2022

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Jun 13, 2022

The behavior of this API is to return the same value as
new Win32Exception(error).Message, while avoiding the
instantiation of an exception.

Fixes #67872

/cc @danmoseley @dotnet/interop-contrib

@dotnet-issue-labeler
Copy link

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 Jun 13, 2022

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

Issue Details

The behavior of this API is to return the same value as
new Win32Exception(error).Message, while avoiding the
instantiation of an exception.

Fixes #67872

/cc @danmoseley @dotnet/interop-contrib

Author: AaronRobinsonMSFT
Assignees: -
Labels:

area-System.Runtime.InteropServices

Milestone: 7.0.0

@stephentoub
Copy link
Member

There are other places using Interop.Kernel32.GetMessage. It'd be nice to have them changed (where possible) to just call Marshal.GetPInvokeErrorMessage, e.g.

string.IsNullOrEmpty(path) ? GetMessage(errorCode) : $"{GetMessage(errorCode)} : '{path}'",

throw new OutOfMemoryException(Interop.Kernel32.GetMessage(errorCode));
default:
throw new ArgumentException(Interop.Kernel32.GetMessage(errorCode));

There are also places doing new Win32Exception().Message that should be changed to use this, e.g.

throw new SecurityException(new Win32Exception().Message);


The behavior of this API is to return the same value as
new Win32Exception(error).Message, while avoiding the
instantion of an exception.
Marshal implementation to handle the P/Invoke call.

Consume new API in places where possible.
@AaronRobinsonMSFT AaronRobinsonMSFT marked this pull request as ready for review June 23, 2022 20:20
@AaronRobinsonMSFT AaronRobinsonMSFT changed the title Implement Marshal.GetPInvokeErrorMessage() API. Implement Marshal.Get(Last)PInvokeErrorMessage() APIs. Jun 23, 2022
Copy link
Member

@danmoseley danmoseley left a comment

Choose a reason for hiding this comment

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

Nice improvement, thanks

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 318018e into dotnet:main Jun 24, 2022
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the runtime_67872 branch June 24, 2022 22:21
@ghost ghost locked as resolved and limited conversation to collaborators Jul 25, 2022
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.

[API Proposal]: Expose method to get system message for system error code
3 participants