Skip to content

Commit

Permalink
Update Marshal.QueryInterface() argument modifier (take 2) (#94470)
Browse files Browse the repository at this point in the history
* Revert "Update Marshal.QueryInterface() argument modifier (#91983)"

This reverts commit a60b66d.

* Update callsites to leverage in Guid parameter

* Use ref in OldDb files (multi-targeting)

* Use ref in more files (multi-targeting)
  • Loading branch information
Sergio0694 authored Jan 2, 2024
1 parent ae051b7 commit acaa83a
Show file tree
Hide file tree
Showing 17 changed files with 53 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ internal static bool TryGetComInstanceForIID(object obj, Guid iid, out IntPtr un
}

wrapperId = wrapper._comWrappers.id;
return Marshal.QueryInterface(wrapper._externalComObject, in iid, out unknown) == HResults.S_OK;
return Marshal.QueryInterface(wrapper._externalComObject, iid, out unknown) == HResults.S_OK;
}

public static unsafe bool TryGetComInstance(object obj, out IntPtr unknown)
Expand All @@ -68,7 +68,7 @@ public static unsafe bool TryGetComInstance(object obj, out IntPtr unknown)
return false;
}

return Marshal.QueryInterface(wrapper._externalComObject, in IID_IUnknown, out unknown) == HResults.S_OK;
return Marshal.QueryInterface(wrapper._externalComObject, IID_IUnknown, out unknown) == HResults.S_OK;
}

public static unsafe bool TryGetObject(IntPtr unknown, [NotNullWhen(true)] out object? obj)
Expand Down Expand Up @@ -512,7 +512,7 @@ static NativeObjectWrapper()
public static NativeObjectWrapper Create(IntPtr externalComObject, IntPtr inner, ComWrappers comWrappers, object comProxy, CreateObjectFlags flags)
{
if (flags.HasFlag(CreateObjectFlags.TrackerObject) &&
Marshal.QueryInterface(externalComObject, in IID_IReferenceTracker, out IntPtr trackerObject) == HResults.S_OK)
Marshal.QueryInterface(externalComObject, IID_IReferenceTracker, out IntPtr trackerObject) == HResults.S_OK)
{
return new ReferenceTrackerNativeObjectWrapper(externalComObject, inner, comWrappers, comProxy, flags, trackerObject);
}
Expand Down Expand Up @@ -854,7 +854,7 @@ public object GetOrRegisterObjectForComInstance(IntPtr externalComObject, Create
{
// It is possible the user has defined their own IUnknown impl so
// we fallback to the tagged interface approach to be sure.
if (0 != Marshal.QueryInterface(comObject, in IID_TaggedImpl, out nint implMaybe))
if (0 != Marshal.QueryInterface(comObject, IID_TaggedImpl, out nint implMaybe))
{
return null;
}
Expand Down Expand Up @@ -888,7 +888,7 @@ private static void DetermineIdentityAndInner(
bool refTrackerInnerScenario = flags.HasFlag(CreateObjectFlags.TrackerObject)
&& flags.HasFlag(CreateObjectFlags.Aggregation);
if (refTrackerInnerScenario &&
Marshal.QueryInterface(externalComObject, in IID_IReferenceTracker, out IntPtr referenceTrackerPtr) == HResults.S_OK)
Marshal.QueryInterface(externalComObject, IID_IReferenceTracker, out IntPtr referenceTrackerPtr) == HResults.S_OK)
{
// We are checking the supplied external value
// for IReferenceTracker since in .NET 5 API usage scenarios
Expand All @@ -900,11 +900,11 @@ private static void DetermineIdentityAndInner(
// be the true identity.
using ComHolder referenceTracker = new ComHolder(referenceTrackerPtr);
checkForIdentity = referenceTrackerPtr;
Marshal.ThrowExceptionForHR(Marshal.QueryInterface(checkForIdentity, in IID_IUnknown, out identity));
Marshal.ThrowExceptionForHR(Marshal.QueryInterface(checkForIdentity, IID_IUnknown, out identity));
}
else
{
Marshal.ThrowExceptionForHR(Marshal.QueryInterface(externalComObject, in IID_IUnknown, out identity));
Marshal.ThrowExceptionForHR(Marshal.QueryInterface(externalComObject, IID_IUnknown, out identity));
}

// Set the inner if scenario dictates an update.
Expand Down Expand Up @@ -1463,7 +1463,7 @@ internal static unsafe int IReferenceTrackerHost_GetTrackerTarget(IntPtr pThis,
return HResults.E_INVALIDARG;
}

if (Marshal.QueryInterface(punk, in IID_IUnknown, out IntPtr ppv) != HResults.S_OK)
if (Marshal.QueryInterface(punk, IID_IUnknown, out IntPtr ppv) != HResults.S_OK)
{
return HResults.COR_E_INVALIDCAST;
}
Expand All @@ -1472,7 +1472,7 @@ internal static unsafe int IReferenceTrackerHost_GetTrackerTarget(IntPtr pThis,
{
using ComHolder identity = new ComHolder(ppv);
using ComHolder trackerTarget = new ComHolder(GetOrCreateTrackerTarget(identity.Ptr));
return Marshal.QueryInterface(trackerTarget.Ptr, in IID_IReferenceTrackerTarget, out *ppNewReference);
return Marshal.QueryInterface(trackerTarget.Ptr, IID_IReferenceTrackerTarget, out *ppNewReference);
}
catch (Exception e)
{
Expand Down Expand Up @@ -1594,7 +1594,7 @@ public static int GetWeakReference(IntPtr pThis, out IntPtr weakReference)
targetPtr != IntPtr.Zero)
{
using ComHolder target = new ComHolder(targetPtr);
if (Marshal.QueryInterface(target.Ptr, in IID_IUnknown, out IntPtr targetIdentityPtr) == HResults.S_OK)
if (Marshal.QueryInterface(target.Ptr, IID_IUnknown, out IntPtr targetIdentityPtr) == HResults.S_OK)
{
using ComHolder targetIdentity = new ComHolder(targetIdentityPtr);
return GetOrCreateObjectFromWrapper(wrapperId, targetIdentity.Ptr);
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Data.OleDb/src/OleDbComWrappers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ protected override object CreateObject(IntPtr externalComObject, CreateObjectFla
Debug.Assert(flags == CreateObjectFlags.UniqueInstance);

Guid errorInfoIID = IID_IErrorInfo;
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
int hr = Marshal.QueryInterface(externalComObject, ref errorInfoIID, out IntPtr comObject);
#pragma warning restore CS9191
if (hr == S_OK)
{
return new ErrorInfoWrapper(comObject);
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Data.OleDb/src/SafeHandles.cs
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,9 @@ internal static unsafe OleDbHResult IChapteredRowsetReleaseChapter(System.IntPtr
finally
{
Guid IID_IChapteredRowset = typeof(System.Data.Common.UnsafeNativeMethods.IChapteredRowset).GUID;
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
hr = (OleDbHResult)Marshal.QueryInterface(ptr, ref IID_IChapteredRowset, out var pChapteredRowset);
#pragma warning restore CS9191
if (pChapteredRowset != IntPtr.Zero)
{
var chapteredRowset = (System.Data.Common.UnsafeNativeMethods.IChapteredRowset)Marshal.GetObjectForIUnknown(pChapteredRowset);
Expand All @@ -696,7 +698,9 @@ internal static unsafe OleDbHResult ITransactionAbort(System.IntPtr ptr)
finally
{
Guid IID_ITransactionLocal = typeof(ITransactionLocal).GUID;
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
hr = (OleDbHResult)Marshal.QueryInterface(ptr, ref IID_ITransactionLocal, out var pTransaction);
#pragma warning restore CS9191
if (pTransaction != IntPtr.Zero)
{
ITransactionLocal transactionLocal = (ITransactionLocal)Marshal.GetObjectForIUnknown(pTransaction);
Expand All @@ -717,7 +721,9 @@ internal static unsafe OleDbHResult ITransactionCommit(System.IntPtr ptr)
finally
{
Guid IID_ITransactionLocal = typeof(ITransactionLocal).GUID;
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
hr = (OleDbHResult)Marshal.QueryInterface(ptr, ref IID_ITransactionLocal, out var pTransaction);
#pragma warning restore CS9191
if (pTransaction != IntPtr.Zero)
{
ITransactionLocal transactionLocal = (ITransactionLocal)Marshal.GetObjectForIUnknown(pTransaction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Runtime.Versioning;
Expand Down Expand Up @@ -1299,8 +1300,8 @@ private static void WorkerThread()
}

// Interfaces that we need to use
private static Guid IID_IObjectContext = new Guid("51372AE0-CAE7-11CF-BE81-00AA00A2FA25");
private static Guid IID_IComThreadingInfo = new Guid("000001ce-0000-0000-C000-000000000046");
private static readonly Guid IID_IObjectContext = new Guid("51372AE0-CAE7-11CF-BE81-00AA00A2FA25");
private static readonly Guid IID_IComThreadingInfo = new Guid("000001ce-0000-0000-C000-000000000046");

// A variable that is initialized once to tell us if we are on
// a Win2k platform or above.
Expand Down Expand Up @@ -1343,7 +1344,9 @@ public static bool IsNoContextMTA()
return false;

// If we CAN get to the IObjectContext interface, we have a 'context'
if (0 == Marshal.QueryInterface(pComThreadingInfo, ref IID_IObjectContext, out pObjectContext))
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
if (0 == Marshal.QueryInterface(pComThreadingInfo, ref Unsafe.AsRef(in IID_IObjectContext), out pObjectContext))
#pragma warning restore CS9191
return false;
}
finally
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ public static IWbemClassObjectFreeThreaded GetErrorInfo()
if (IntPtr.Zero != pErrorInfo && new IntPtr(-1) != pErrorInfo)
{
IntPtr pIWbemClassObject;
#pragma warning disable CS9191 // The 'ref' modifier for argument 1 corresponding to 'in' parameter is equivalent to 'in'. Consider using 'in' instead.
Marshal.QueryInterface(pErrorInfo, ref IWbemClassObjectFreeThreaded.IID_IWbemClassObject, out pIWbemClassObject);
#pragma warning restore CS9191
Marshal.Release(pErrorInfo);

// The IWbemClassObjectFreeThreaded instance will own reference count on pIWbemClassObject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public static int SizeOf<T>()
return SizeOfHelper(t, throwIfNotMarshalable: true);
}

public static unsafe int QueryInterface(IntPtr pUnk, ref readonly Guid iid, out IntPtr ppv)
public static unsafe int QueryInterface(IntPtr pUnk, in Guid iid, out IntPtr ppv)
{
ArgumentNullException.ThrowIfNull(pUnk);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ public static void PtrToStructure(System.IntPtr ptr, object structure) { }
public static object? PtrToStructure(System.IntPtr ptr, [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicConstructors| System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)] System.Type structureType) { throw null; }
public static T? PtrToStructure<[System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.NonPublicConstructors | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicConstructors)]T>(System.IntPtr ptr) { throw null; }
public static void PtrToStructure<T>(System.IntPtr ptr, [System.Diagnostics.CodeAnalysis.DisallowNullAttribute] T structure) { }
public static int QueryInterface(System.IntPtr pUnk, ref readonly System.Guid iid, out System.IntPtr ppv) { throw null; }
public static int QueryInterface(System.IntPtr pUnk, in System.Guid iid, out System.IntPtr ppv) { throw null; }
public static byte ReadByte(System.IntPtr ptr) { throw null; }
public static byte ReadByte(System.IntPtr ptr, int ofs) { throw null; }
[System.Diagnostics.CodeAnalysis.RequiresDynamicCode("Marshalling code for the object might not be available")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal sealed unsafe class FreeThreadedStrategy : IIUnknownStrategy

unsafe int IIUnknownStrategy.QueryInterface(void* thisPtr, in Guid handle, out void* ppObj)
{
int hr = Marshal.QueryInterface((nint)thisPtr, in handle, out nint ppv);
int hr = Marshal.QueryInterface((nint)thisPtr, handle, out nint ppv);
if (hr < 0)
{
ppObj = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public unsafe interface IIUnknownStrategy
/// <param name="iid">The IID (Interface ID) to query for.</param>
/// <param name="ppObj">The resulting interface.</param>
/// <returns>Returns an HRESULT represents the success of the operation.</returns>
/// <seealso cref="Marshal.QueryInterface(nint, ref readonly Guid, out nint)"/>
/// <seealso cref="Marshal.QueryInterface(nint, in Guid, out nint)"/>
public int QueryInterface(void* instancePtr, in Guid iid, out void* ppObj);

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ public void ComInstanceProvidesInterfaceForDirectlyImplementedComInterface()
StrategyBasedComWrappers wrappers = new();
nint ptr = wrappers.GetOrCreateComInterfaceForObject(obj, CreateComInterfaceFlags.None);
Assert.NotEqual(0, ptr);
var iid = typeof(IGetAndSetInt).GUID;
Assert.Equal(0, Marshal.QueryInterface(ptr, in iid, out nint iComInterface));
Assert.Equal(0, Marshal.QueryInterface(ptr, typeof(IGetAndSetInt).GUID, out nint iComInterface));
Assert.NotEqual(0, iComInterface);
Marshal.Release(iComInterface);
Marshal.Release(ptr);
Expand All @@ -60,8 +59,7 @@ public void ComInstanceProvidesInterfaceForIndirectlyImplementedComInterface()
StrategyBasedComWrappers wrappers = new();
nint ptr = wrappers.GetOrCreateComInterfaceForObject(obj, CreateComInterfaceFlags.None);
Assert.NotEqual(0, ptr);
var iid = typeof(IGetAndSetInt).GUID;
Assert.Equal(0, Marshal.QueryInterface(ptr, in iid, out nint iComInterface));
Assert.Equal(0, Marshal.QueryInterface(ptr, typeof(IGetAndSetInt).GUID, out nint iComInterface));
Assert.NotEqual(0, iComInterface);
Marshal.Release(iComInterface);
Marshal.Release(ptr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public void QueryInterface_ValidComObjectInterface_Success(object o, string iidS
try
{
Guid guid = new Guid(iidString);
Assert.Equal(0, Marshal.QueryInterface(ptr, in guid, out IntPtr ppv));
Assert.Equal(0, Marshal.QueryInterface(ptr, guid, out IntPtr ppv));
Assert.NotEqual(IntPtr.Zero, ppv);
try
{
Expand Down Expand Up @@ -103,7 +103,7 @@ public void QueryInterface_NoSuchComObjectInterface_Success(object o, string iid
try
{
Guid iid = new Guid(iidString);
Assert.Equal(E_NOINTERFACE, Marshal.QueryInterface(ptr, in iid, out IntPtr ppv));
Assert.Equal(E_NOINTERFACE, Marshal.QueryInterface(ptr, iid, out IntPtr ppv));
Assert.Equal(IntPtr.Zero, ppv);
Assert.Equal(new Guid(iidString), iid);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public void QueryInterface_ValidInterface_Success(object o, string iidString)
try
{
Guid guid = new Guid(iidString);
Assert.Equal(0, Marshal.QueryInterface(ptr, in guid, out IntPtr ppv));
Assert.Equal(0, Marshal.QueryInterface(ptr, guid, out IntPtr ppv));
Assert.NotEqual(IntPtr.Zero, ppv);
try
{
Expand Down Expand Up @@ -64,7 +64,7 @@ public void QueryInterface_NoSuchInterface_Success(object o, string iidString)
try
{
Guid iid = new Guid(iidString);
Assert.Equal(E_NOINTERFACE, Marshal.QueryInterface(ptr, in iid, out IntPtr ppv));
Assert.Equal(E_NOINTERFACE, Marshal.QueryInterface(ptr, iid, out IntPtr ppv));
Assert.Equal(IntPtr.Zero, ppv);
Assert.Equal(new Guid(iidString), iid);
}
Expand All @@ -77,8 +77,7 @@ public void QueryInterface_NoSuchInterface_Success(object o, string iidString)
[Fact]
public void QueryInterface_ZeroPointer_ThrowsArgumentNullException()
{
Guid iid = Guid.Empty;
AssertExtensions.Throws<ArgumentNullException>("pUnk", () => Marshal.QueryInterface(IntPtr.Zero, in iid, out IntPtr ppv));
AssertExtensions.Throws<ArgumentNullException>("pUnk", () => Marshal.QueryInterface(IntPtr.Zero, Guid.Empty, out IntPtr ppv));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ interface IComInterface1

public void SetData(int x);

public static Guid IID = new Guid("2c3f9903-b586-46b1-881b-adfce9af47b1");
public static readonly Guid IID = new Guid("2c3f9903-b586-46b1-881b-adfce9af47b1");
}

// Call from another assembly to get a ptr to make an RCW
Expand Down Expand Up @@ -100,7 +100,7 @@ static ComInterfaceEntry* MyObjectComInterfaceEntries
}
protected override object CreateObject(nint ptr, CreateObjectFlags flags)
{
int hr = Marshal.QueryInterface(ptr, in IComInterface1.IID, out IntPtr IComInterfaceImpl);
int hr = Marshal.QueryInterface(ptr, IComInterface1.IID, out IntPtr IComInterfaceImpl);
if (hr != 0)
{
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ internal static ITransaction GetITransactionFromIDtcTransaction(IDtcTransaction
unsafe
{
nint unknown = Marshal.GetIUnknownForObject(transactionNative);
if (Marshal.QueryInterface(unknown, in Guids.IID_ITransaction_Guid, out IntPtr transactionNativePtr) == 0)
if (Marshal.QueryInterface(unknown, Guids.IID_ITransaction_Guid, out IntPtr transactionNativePtr) == 0)
{
Marshal.Release(unknown);
myTransactionNative = ComInterfaceMarshaller<ITransaction>.ConvertToManaged((void*)transactionNativePtr)!;
Expand Down
Loading

0 comments on commit acaa83a

Please sign in to comment.