From 6d6a373824df45eb1121097945ff0bac4c009385 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 17 Jun 2022 08:43:13 -0700 Subject: [PATCH] Replace a few instances of PtrToStructure with more efficient marshalling (#70866) --- .../Interop/Windows/Crypt32/Interop.FindOidInfo.cs | 22 ++--- .../Interop/Windows/Crypt32/Interop.HashIdAlg.cs | 21 ++--- .../Windows/IpHlpApi/Interop.NetworkInformation.cs | 103 ++++++++++++--------- .../System/Diagnostics/FileVersionInfo.Windows.cs | 4 +- .../SystemIPInterfaceProperties.cs | 4 +- .../SystemIPv4InterfaceProperties.cs | 7 +- .../NetworkInformation/SystemNetworkInterface.cs | 25 ++--- .../SystemUnicastIPAddressInformation.cs | 12 +-- .../src/System/Security/Principal/SID.cs | 4 +- 9 files changed, 101 insertions(+), 101 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.FindOidInfo.cs b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.FindOidInfo.cs index 8733d41..79e9701 100644 --- a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.FindOidInfo.cs +++ b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.FindOidInfo.cs @@ -48,7 +48,7 @@ internal static partial class Interop CRYPT_OID_INFO_CNG_SIGN_KEY = 6, } - internal static CRYPT_OID_INFO FindOidInfo(CryptOidInfoKeyType keyType, string key, OidGroup group, bool fallBackToAllGroups) + internal static unsafe CRYPT_OID_INFO FindOidInfo(CryptOidInfoKeyType keyType, string key, OidGroup group, bool fallBackToAllGroups) { const OidGroup CRYPT_OID_DISABLE_SEARCH_DS_FLAG = unchecked((OidGroup)0x80000000); Debug.Assert(key != null); @@ -75,28 +75,28 @@ internal static partial class Interop if (!OidGroupWillNotUseActiveDirectory(group)) { OidGroup localGroup = group | CRYPT_OID_DISABLE_SEARCH_DS_FLAG; - IntPtr localOidInfo = CryptFindOIDInfo(keyType, rawKey, localGroup); - if (localOidInfo != IntPtr.Zero) + CRYPT_OID_INFO* localOidInfo = CryptFindOIDInfo(keyType, rawKey, localGroup); + if (localOidInfo != null) { - return Marshal.PtrToStructure(localOidInfo); + return *localOidInfo; } } // Attempt to query with a specific group, to make try to avoid an AD lookup if possible - IntPtr fullOidInfo = CryptFindOIDInfo(keyType, rawKey, group); - if (fullOidInfo != IntPtr.Zero) + CRYPT_OID_INFO* fullOidInfo = CryptFindOIDInfo(keyType, rawKey, group); + if (fullOidInfo != null) { - return Marshal.PtrToStructure(fullOidInfo); + return *fullOidInfo; } if (fallBackToAllGroups && group != OidGroup.All) { // Finally, for compatibility with previous runtimes, if we have a group specified retry the // query with no group - IntPtr allGroupOidInfo = CryptFindOIDInfo(keyType, rawKey, OidGroup.All); - if (allGroupOidInfo != IntPtr.Zero) + CRYPT_OID_INFO* allGroupOidInfo = CryptFindOIDInfo(keyType, rawKey, OidGroup.All); + if (allGroupOidInfo != null) { - return Marshal.PtrToStructure(allGroupOidInfo); + return *allGroupOidInfo; } } @@ -125,6 +125,6 @@ internal static partial class Interop } [LibraryImport(Interop.Libraries.Crypt32)] - private static partial IntPtr CryptFindOIDInfo(CryptOidInfoKeyType dwKeyType, IntPtr pvKey, OidGroup group); + private static unsafe partial CRYPT_OID_INFO* CryptFindOIDInfo(CryptOidInfoKeyType dwKeyType, IntPtr pvKey, OidGroup group); } } diff --git a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.HashIdAlg.cs b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.HashIdAlg.cs index 4d5110c..d7dfe86 100644 --- a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.HashIdAlg.cs +++ b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.HashIdAlg.cs @@ -13,23 +13,18 @@ internal static partial class Interop /// Version used for a buffer containing a scalar integer (not an IntPtr) /// [LibraryImport(Libraries.Crypt32)] - private static unsafe partial IntPtr CryptFindOIDInfo(CryptOidInfoKeyType dwKeyType, int* pvKey, OidGroup group); + private static unsafe partial CRYPT_OID_INFO* CryptFindOIDInfo(CryptOidInfoKeyType dwKeyType, void* pvKey, OidGroup group); - public static CRYPT_OID_INFO FindAlgIdOidInfo(Interop.BCrypt.ECC_CURVE_ALG_ID_ENUM algId) + public static unsafe CRYPT_OID_INFO FindAlgIdOidInfo(Interop.BCrypt.ECC_CURVE_ALG_ID_ENUM algId) { - int intAlgId = (int)algId; - IntPtr fullOidInfo; - unsafe - { - fullOidInfo = CryptFindOIDInfo( - CryptOidInfoKeyType.CRYPT_OID_INFO_ALGID_KEY, - &intAlgId, - OidGroup.HashAlgorithm); - } + CRYPT_OID_INFO* fullOidInfo = CryptFindOIDInfo( + CryptOidInfoKeyType.CRYPT_OID_INFO_ALGID_KEY, + &algId, + OidGroup.HashAlgorithm); - if (fullOidInfo != IntPtr.Zero) + if (fullOidInfo != null) { - return Marshal.PtrToStructure(fullOidInfo); + return *fullOidInfo; } // Otherwise the lookup failed. diff --git a/src/libraries/Common/src/Interop/Windows/IpHlpApi/Interop.NetworkInformation.cs b/src/libraries/Common/src/Interop/Windows/IpHlpApi/Interop.NetworkInformation.cs index 11b32ab..3f0cd74 100644 --- a/src/libraries/Common/src/Interop/Windows/IpHlpApi/Interop.NetworkInformation.cs +++ b/src/libraries/Common/src/Interop/Windows/IpHlpApi/Interop.NetworkInformation.cs @@ -76,24 +76,22 @@ internal static partial class Interop // IP_ADAPTER_WINS_SERVER_ADDRESS // IP_ADAPTER_GATEWAY_ADDRESS [StructLayout(LayoutKind.Sequential)] - internal struct IpAdapterAddress + internal unsafe struct IpAdapterAddress { internal uint length; internal AdapterAddressFlags flags; - internal IntPtr next; + internal IpAdapterAddress* next; internal IpSocketAddress address; internal static InternalIPAddressCollection MarshalIpAddressCollection(IntPtr ptr) { InternalIPAddressCollection addressList = new InternalIPAddressCollection(); - while (ptr != IntPtr.Zero) + IpAdapterAddress* pIpAdapterAddress = (IpAdapterAddress*)ptr; + while (pIpAdapterAddress != null) { - IpAdapterAddress addressStructure = Marshal.PtrToStructure(ptr); - IPAddress address = addressStructure.address.MarshalIPAddress(); - addressList.InternalAdd(address); - - ptr = addressStructure.next; + addressList.InternalAdd(pIpAdapterAddress->address.MarshalIPAddress()); + pIpAdapterAddress = pIpAdapterAddress->next; } return addressList; @@ -103,13 +101,12 @@ internal static partial class Interop { IPAddressInformationCollection addressList = new IPAddressInformationCollection(); - while (ptr != IntPtr.Zero) + IpAdapterAddress* pIpAdapterAddress = (IpAdapterAddress*)ptr; + while (pIpAdapterAddress != null) { - IpAdapterAddress addressStructure = Marshal.PtrToStructure(ptr); - IPAddress address = addressStructure.address.MarshalIPAddress(); - addressList.InternalAdd(new SystemIPAddressInformation(address, addressStructure.flags)); - - ptr = addressStructure.next; + addressList.InternalAdd(new SystemIPAddressInformation( + pIpAdapterAddress->address.MarshalIPAddress(), pIpAdapterAddress->flags)); + pIpAdapterAddress = pIpAdapterAddress->next; } return addressList; @@ -117,11 +114,11 @@ internal static partial class Interop } [StructLayout(LayoutKind.Sequential)] - internal struct IpAdapterUnicastAddress + internal unsafe struct IpAdapterUnicastAddress { internal uint length; internal AdapterAddressFlags flags; - internal IntPtr next; + internal IpAdapterUnicastAddress* next; internal IpSocketAddress address; internal PrefixOrigin prefixOrigin; internal SuffixOrigin suffixOrigin; @@ -132,37 +129,59 @@ internal static partial class Interop internal byte prefixLength; } - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)] - internal struct IpAdapterAddresses + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct IpAdapterAddresses { internal const int MAX_ADAPTER_ADDRESS_LENGTH = 8; internal uint length; internal uint index; - internal IntPtr next; + internal IpAdapterAddresses* next; - // Needs to be ANSI. - [MarshalAs(UnmanagedType.LPStr)] - internal string AdapterName; + private IntPtr _adapterName; // ANSI string + internal string AdapterName => Marshal.PtrToStringAnsi(_adapterName)!; internal IntPtr firstUnicastAddress; internal IntPtr firstAnycastAddress; internal IntPtr firstMulticastAddress; internal IntPtr firstDnsServerAddress; - internal string dnsSuffix; - internal string description; - internal string friendlyName; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = MAX_ADAPTER_ADDRESS_LENGTH)] - internal byte[] address; - internal uint addressLength; + private IntPtr _dnsSuffix; + internal string DnsSuffix => Marshal.PtrToStringUni(_dnsSuffix)!; + + private IntPtr _description; + internal string Description => Marshal.PtrToStringUni(_description)!; + + private IntPtr _friendlyName; + internal string FriendlyName => Marshal.PtrToStringUni(_friendlyName)!; + + private fixed byte _address[MAX_ADAPTER_ADDRESS_LENGTH]; + private uint _addressLength; + internal byte[] Address + { + get + { + fixed (byte* pAddress = _address) + return new ReadOnlySpan(pAddress, (int)_addressLength).ToArray(); + } + } + internal AdapterFlags flags; internal uint mtu; internal NetworkInterfaceType type; internal OperationalStatus operStatus; internal uint ipv6Index; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 16)] - internal uint[] zoneIndices; + + private fixed uint _zoneIndices[16]; + internal uint[] ZoneIndices + { + get + { + fixed (uint* pZoneIndices = _zoneIndices) + return new ReadOnlySpan(pZoneIndices, 16).ToArray(); + } + } + internal IntPtr firstPrefix; internal ulong transmitLinkSpeed; @@ -174,13 +193,11 @@ internal static partial class Interop internal ulong luid; internal IpSocketAddress dhcpv4Server; internal uint compartmentId; - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 16)] - internal byte[] networkGuid; + internal fixed byte networkGuid[16]; internal InterfaceConnectionType connectionType; internal InterfaceTunnelType tunnelType; internal IpSocketAddress dhcpv6Server; // Never available in Windows. - [MarshalAs(UnmanagedType.ByValArray, SizeConst = 130)] - internal byte[] dhcpv6ClientDuid; + internal fixed byte dhcpv6ClientDuid[130]; internal uint dhcpv6ClientDuidLength; internal uint dhcpV6Iaid; @@ -211,11 +228,11 @@ internal static partial class Interop /// /// IP_PER_ADAPTER_INFO - per-adapter IP information such as DNS server list. /// - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + [StructLayout(LayoutKind.Sequential)] internal struct IpPerAdapterInfo { - internal bool autoconfigEnabled; - internal bool autoconfigActive; + internal uint autoconfigEnabled; + internal uint autoconfigActive; internal IntPtr currentDnsServer; /* IpAddressList* */ internal IpAddrString dnsServerList; }; @@ -224,14 +241,12 @@ internal static partial class Interop /// Store an IP address with its corresponding subnet mask, /// both as dotted decimal strings. /// - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] - internal struct IpAddrString + [StructLayout(LayoutKind.Sequential)] + internal unsafe struct IpAddrString { - internal IntPtr Next; /* struct _IpAddressList* */ - [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 16)] - internal string IpAddress; - [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 16)] - internal string IpMask; + internal IpAddrString* Next; /* struct _IpAddressList* */ + internal fixed byte IpAddress[16]; + internal fixed byte IpMask[16]; internal uint Context; } diff --git a/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Windows.cs b/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Windows.cs index 4b57517..fa5234b 100644 --- a/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Windows.cs +++ b/src/libraries/System.Diagnostics.FileVersionInfo/src/System/Diagnostics/FileVersionInfo.Windows.cs @@ -68,11 +68,11 @@ namespace System.Diagnostics return value.ToString("X8"); } - private static Interop.Version.VS_FIXEDFILEINFO GetFixedFileInfo(IntPtr memPtr) + private static unsafe Interop.Version.VS_FIXEDFILEINFO GetFixedFileInfo(IntPtr memPtr) { if (Interop.Version.VerQueryValue(memPtr, "\\", out IntPtr memRef, out _)) { - return (Interop.Version.VS_FIXEDFILEINFO)Marshal.PtrToStructure(memRef); + return *(Interop.Version.VS_FIXEDFILEINFO*)memRef; } return default; diff --git a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPInterfaceProperties.cs b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPInterfaceProperties.cs index 6d261115..fbdd43d 100644 --- a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPInterfaceProperties.cs +++ b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPInterfaceProperties.cs @@ -29,7 +29,7 @@ namespace System.Net.NetworkInformation internal SystemIPInterfaceProperties(in Interop.IpHlpApi.FIXED_INFO fixedInfo, in Interop.IpHlpApi.IpAdapterAddresses ipAdapterAddresses) { _adapterFlags = ipAdapterAddresses.flags; - _dnsSuffix = ipAdapterAddresses.dnsSuffix; + _dnsSuffix = ipAdapterAddresses.DnsSuffix; _dnsEnabled = fixedInfo.enableDns; _dynamicDnsEnabled = ((ipAdapterAddresses.flags & Interop.IpHlpApi.AdapterFlags.DnsEnabled) > 0); @@ -64,7 +64,7 @@ namespace System.Net.NetworkInformation if ((_adapterFlags & Interop.IpHlpApi.AdapterFlags.IPv6Enabled) != 0) { _ipv6Properties = new SystemIPv6InterfaceProperties(ipAdapterAddresses.ipv6Index, - ipAdapterAddresses.mtu, ipAdapterAddresses.zoneIndices); + ipAdapterAddresses.mtu, ipAdapterAddresses.ZoneIndices); } } diff --git a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPv4InterfaceProperties.cs b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPv4InterfaceProperties.cs index 7c455b4..e96e1cd 100644 --- a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPv4InterfaceProperties.cs +++ b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemIPv4InterfaceProperties.cs @@ -91,11 +91,10 @@ namespace System.Net.NetworkInformation result = Interop.IpHlpApi.GetPerAdapterInfo(index, buffer, &size); if (result == Interop.IpHlpApi.ERROR_SUCCESS) { - Interop.IpHlpApi.IpPerAdapterInfo ipPerAdapterInfo = - Marshal.PtrToStructure(buffer); + Interop.IpHlpApi.IpPerAdapterInfo* ipPerAdapterInfo = (Interop.IpHlpApi.IpPerAdapterInfo*)buffer; - _autoConfigEnabled = ipPerAdapterInfo.autoconfigEnabled; - _autoConfigActive = ipPerAdapterInfo.autoconfigActive; + _autoConfigEnabled = ipPerAdapterInfo->autoconfigEnabled != 0; + _autoConfigActive = ipPerAdapterInfo->autoconfigActive != 0; } } finally diff --git a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemNetworkInterface.cs b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemNetworkInterface.cs index ed80678..e04e9e6 100644 --- a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemNetworkInterface.cs +++ b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemNetworkInterface.cs @@ -15,7 +15,6 @@ namespace System.Net.NetworkInformation private readonly string _id; private readonly string _description; private readonly byte[] _physicalAddress; - private readonly uint _addressLength; private readonly NetworkInterfaceType _type; private readonly OperationalStatus _operStatus; private readonly long _speed; @@ -110,14 +109,12 @@ namespace System.Net.NetworkInformation if (result == Interop.IpHlpApi.ERROR_SUCCESS) { // Linked list of interfaces. - IntPtr ptr = buffer; - while (ptr != IntPtr.Zero) + Interop.IpHlpApi.IpAdapterAddresses* adapterAddresses = (Interop.IpHlpApi.IpAdapterAddresses*)buffer; + while (adapterAddresses != null) { // Traverse the list, marshal in the native structures, and create new NetworkInterfaces. - Interop.IpHlpApi.IpAdapterAddresses adapterAddresses = Marshal.PtrToStructure(ptr); - interfaceList.Add(new SystemNetworkInterface(in fixedInfo, in adapterAddresses)); - - ptr = adapterAddresses.next; + interfaceList.Add(new SystemNetworkInterface(in fixedInfo, in *adapterAddresses)); + adapterAddresses = adapterAddresses->next; } } } @@ -146,12 +143,11 @@ namespace System.Net.NetworkInformation { // Store the common API information. _id = ipAdapterAddresses.AdapterName; - _name = ipAdapterAddresses.friendlyName; - _description = ipAdapterAddresses.description; + _name = ipAdapterAddresses.FriendlyName; + _description = ipAdapterAddresses.Description; _index = ipAdapterAddresses.index; - _physicalAddress = ipAdapterAddresses.address; - _addressLength = ipAdapterAddresses.addressLength; + _physicalAddress = ipAdapterAddresses.Address; _type = ipAdapterAddresses.type; _operStatus = ipAdapterAddresses.operStatus; @@ -172,12 +168,7 @@ namespace System.Net.NetworkInformation public override PhysicalAddress GetPhysicalAddress() { - byte[] newAddr = new byte[_addressLength]; - - // Buffer.BlockCopy only supports int while addressLength is uint (see IpAdapterAddresses). - // Will throw OverflowException if addressLength > Int32.MaxValue. - Buffer.BlockCopy(_physicalAddress, 0, newAddr, 0, checked((int)_addressLength)); - return new PhysicalAddress(newAddr); + return new PhysicalAddress(_physicalAddress); } public override NetworkInterfaceType NetworkInterfaceType { get { return _type; } } diff --git a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemUnicastIPAddressInformation.cs b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemUnicastIPAddressInformation.cs index e9ed56f..774d3f2 100644 --- a/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemUnicastIPAddressInformation.cs +++ b/src/libraries/System.Net.NetworkInformation/src/System/Net/NetworkInformation/SystemUnicastIPAddressInformation.cs @@ -20,7 +20,7 @@ namespace System.Net.NetworkInformation private readonly uint _preferredLifetime; private readonly byte _prefixLength; - internal SystemUnicastIPAddressInformation(Interop.IpHlpApi.IpAdapterUnicastAddress adapterAddress) + internal SystemUnicastIPAddressInformation(in Interop.IpHlpApi.IpAdapterUnicastAddress adapterAddress) { IPAddress ipAddress = adapterAddress.address.MarshalIPAddress(); _innerInfo = new SystemIPAddressInformation(ipAddress, adapterAddress.flags); @@ -135,15 +135,15 @@ namespace System.Net.NetworkInformation } // Helper method that marshals the address information into the classes. - internal static UnicastIPAddressInformationCollection MarshalUnicastIpAddressInformationCollection(IntPtr ptr) + internal static unsafe UnicastIPAddressInformationCollection MarshalUnicastIpAddressInformationCollection(IntPtr ptr) { UnicastIPAddressInformationCollection addressList = new UnicastIPAddressInformationCollection(); - while (ptr != IntPtr.Zero) + Interop.IpHlpApi.IpAdapterUnicastAddress* pIpAdapterAddress = (Interop.IpHlpApi.IpAdapterUnicastAddress*)ptr; + while (pIpAdapterAddress != null) { - Interop.IpHlpApi.IpAdapterUnicastAddress addr = Marshal.PtrToStructure(ptr); - addressList.InternalAdd(new SystemUnicastIPAddressInformation(addr)); - ptr = addr.next; + addressList.InternalAdd(new SystemUnicastIPAddressInformation(in *pIpAdapterAddress)); + pIpAdapterAddress = pIpAdapterAddress->next; } return addressList; diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs index 5fc2348..8b16387 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/SID.cs @@ -1034,8 +1034,8 @@ namespace System.Security.Principal for (int i = 0; i < rdl.Entries; i++) { - Interop.LSA_TRUST_INFORMATION ti = (Interop.LSA_TRUST_INFORMATION)Marshal.PtrToStructure(new IntPtr((long)rdl.Domains + i * Marshal.SizeOf())); - ReferencedDomains[i] = Marshal.PtrToStringUni(ti.Name.Buffer, ti.Name.Length / sizeof(char)); + Interop.LSA_TRUST_INFORMATION* ti = (Interop.LSA_TRUST_INFORMATION*)rdl.Domains + i; + ReferencedDomains[i] = Marshal.PtrToStringUni(ti->Name.Buffer, ti->Name.Length / sizeof(char)); } Interop.LSA_TRANSLATED_NAME[] translatedNames = new Interop.LSA_TRANSLATED_NAME[sourceSids.Count]; -- 2.7.4