-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Use DllImportGenerator in System.Diagnostics.PerformanceCounter #61389
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,8 +8,7 @@ internal static partial class Interop | |
{ | ||
internal static partial class Advapi32 | ||
{ | ||
[DllImport(Interop.Libraries.Advapi32, EntryPoint = "GetSecurityDescriptorLength", CallingConvention = CallingConvention.Winapi, | ||
CharSet = CharSet.Unicode, ExactSpelling = true)] | ||
internal static extern /*DWORD*/ uint GetSecurityDescriptorLength(IntPtr byteArray); | ||
[GeneratedDllImport(Libraries.Advapi32, EntryPoint = "GetSecurityDescriptorLength", CharSet = CharSet.Unicode, ExactSpelling = true)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we are just removing CallingConvention = CallingConvention.Winapi from everywhere as it's default and archaic. |
||
internal static partial uint GetSecurityDescriptorLength(IntPtr byteArray); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,39 +9,24 @@ internal static partial class Interop | |
{ | ||
internal static partial class Advapi32 | ||
{ | ||
#if DLLIMPORTGENERATOR_ENABLED | ||
[GeneratedDllImport(Interop.Libraries.Advapi32, SetLastError = true)] | ||
internal static partial bool GetTokenInformation( | ||
#else | ||
[DllImport(Interop.Libraries.Advapi32, SetLastError = true)] | ||
internal static extern bool GetTokenInformation( | ||
#endif | ||
SafeAccessTokenHandle TokenHandle, | ||
uint TokenInformationClass, | ||
SafeLocalAllocHandle TokenInformation, | ||
uint TokenInformationLength, | ||
out uint ReturnLength); | ||
|
||
#if DLLIMPORTGENERATOR_ENABLED | ||
[GeneratedDllImport(Interop.Libraries.Advapi32, SetLastError = true)] | ||
internal static partial bool GetTokenInformation( | ||
#else | ||
[DllImport(Interop.Libraries.Advapi32, SetLastError = true)] | ||
internal static extern bool GetTokenInformation( | ||
#endif | ||
IntPtr TokenHandle, | ||
uint TokenInformationClass, | ||
SafeLocalAllocHandle TokenInformation, | ||
uint TokenInformationLength, | ||
out uint ReturnLength); | ||
|
||
#if DLLIMPORTGENERATOR_ENABLED | ||
[GeneratedDllImport(Interop.Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] | ||
[GeneratedDllImport(Interop.Libraries.Advapi32, SetLastError = true)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why remove CharSet = CharSet.Unicode ? is it the default now? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not the default. I removed it because it is not necessary for this p/invoke - no string data being marshalled, no A/W suffix variants - and having it set to Unicode in this case makes it so that we do an extra probe for an entry point named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this seems like a great thing for the generator to warn on (I say the generator since it's new so it wouldn't be a breaking change, and folks are touching their sources anyway) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're currently discussing how we can make the whole character set situation better with the generator approach: #61326. One thing we are considering as part of that is removing ExactSpelling (so weird/negative implications of char set like this wouldn't be a thing) and effectively requiring it to always be true. |
||
internal static partial bool GetTokenInformation( | ||
#else | ||
[DllImport(Interop.Libraries.Advapi32, CharSet = CharSet.Unicode, SetLastError = true)] | ||
internal static extern bool GetTokenInformation( | ||
#endif | ||
IntPtr TokenHandle, | ||
uint TokenInformationClass, | ||
IntPtr TokenInformation, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,11 +13,11 @@ internal static partial class Interop | |
{ | ||
internal static partial class Advapi32 | ||
{ | ||
[DllImport(Libraries.Advapi32, CharSet = CharSet.Unicode, BestFitMapping = false, EntryPoint = "RegEnumValueW", ExactSpelling = true)] | ||
internal static extern int RegEnumValue( | ||
[GeneratedDllImport(Libraries.Advapi32, EntryPoint = "RegEnumValueW", CharSet = CharSet.Unicode, ExactSpelling = true)] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we assuming BestFitMapping = true is fine in these? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, these are all using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like there are several variations of DllImport/GeneratedDllImport properties that are not meaningful or invalid but just ignored. I wonder whether it would make sense for the generator to flag/reject those so the ycan be cleaned up during conversion? I guess it is a minor thing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Certainly something we should keep in mind. We've been trying to remove support for sometimes-non-meaningful things (like |
||
internal static partial int RegEnumValue( | ||
SafeRegistryHandle hKey, | ||
int dwIndex, | ||
char[] lpValueName, | ||
[Out] char[] lpValueName, | ||
ref int lpcbValueName, | ||
IntPtr lpReserved_MustBeZero, | ||
int[]? lpType, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bah! Sorry @jkoritzinsky looks like I did miss that case <shame />