Remove [Out] by-value string parameter usage from System.Management. #34091
Conversation
jkoritzinsky
requested review from
stephentoub,
Anipik and
AaronRobinsonMSFT
There was a problem hiding this comment.
Thanks, but I just finished removing all use of [Out] StringBuilder marshaling from coreclr and corefx, and I would like to avoid adding any additional back in (these also don't appear to actually need [In]). I'd like to see these defined without using StringBuilder, e.g. using char[] or even char*.
src/System.Management/src/System/Management/InteropClasses/WMIInterop.cs
Outdated
Show resolved
Hide resolved
src/System.Management/src/System/Management/InteropClasses/WMIInterop.cs
Outdated
Show resolved
Hide resolved
|
The tests for this library are essentially manual. We should do a sanity test. |
|
@stephentoub can you take another look at this PR when you have a chance? |
| @@ -948,13 +949,13 @@ interface IWbemPathKeyList | |||
| [PreserveSig] int GetCount_([Out] out uint puKeyCount); | |||
| [PreserveSig] int SetKey_([In][MarshalAs(UnmanagedType.LPWStr)] string wszName, [In] uint uFlags, [In] uint uCimType, [In] IntPtr pKeyVal); | |||
| [PreserveSig] int SetKey2_([In][MarshalAs(UnmanagedType.LPWStr)] string wszName, [In] uint uFlags, [In] uint uCimType, [In] ref object pKeyVal); | |||
| [PreserveSig] int GetKey_([In] uint uKeyIx, [In] uint uFlags, [In][Out] ref uint puNameBufSize, [In][Out][MarshalAs(UnmanagedType.LPWStr)] string pszKeyName, [In][Out] ref uint puKeyValBufSize, [In][Out] IntPtr pKeyVal, [Out] out uint puApparentCimType); | |||
| [PreserveSig] int GetKey2_([In] uint uKeyIx, [In] uint uFlags, [In][Out] ref uint puNameBufSize, [In][Out][MarshalAs(UnmanagedType.LPWStr)] string pszKeyName, [In][Out] ref object pKeyValue, [Out] out uint puApparentCimType); | |||
| [PreserveSig] int GetKey_([In] uint uKeyIx, [In] uint uFlags, [In][Out] ref uint puNameBufSize, [Out][MarshalAs(UnmanagedType.LPArray, ArraySubType = UnmanagedType.U2, SizeParamIndex = 2)] char[] pszKeyName, [In][Out] ref uint puKeyValBufSize, [In][Out] IntPtr pKeyVal, [Out] out uint puApparentCimType); | |||
There was a problem hiding this comment.
Is this MarshalAs necessary? What impact does it have on behavior?
There was a problem hiding this comment.
The MarshalAs makes it explicit that we're using Unicode characters. Without it, it was marshalling the array as an ANSI array.
| status = wbemPath.GetText_(flags, ref bufLen, pathStr); | ||
| char[] pathChars = new char[(int)bufLen]; | ||
| status = wbemPath.GetText_(flags, ref bufLen, pathChars); | ||
| pathStr = new string(pathChars, 0, Array.IndexOf(pathChars, '\0')); |
There was a problem hiding this comment.
This is functionally the same as what was being done previously with StringBuilder, in that StringBuilder marshaling will search for the '\0' and use that when copying back the data. We could make this more efficient presumably by using bufLen-1 instead if status indicates success, but I'm guessing we're not particularly concerned with performance in this code, and our testing isn't great around this stuff, so it's probably fine as-is.
|
I've done some sanity checks locally. I'm going to merge this in. |
Fixes #34083.