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

SafeHandle overload should be generated for functions with out parameters #1183

Open
AArnott opened this issue May 13, 2024 · 5 comments
Open

Comments

@AArnott
Copy link
Member

AArnott commented May 13, 2024

What I don't see here is an actual SafeHandle-derived type being emitted by CsWin32 even when the allocating InitializeProcThreadAttributeList function is generated. This is unexpected, and is probably due to InitializeProcThreadAttributeList using an output parameter to provide the handle to its caller instead of its return value. CsWin32 may not be prepared to trigger its SafeHandle overload and type for allocating functions except by return value.

Originally posted by @AArnott in #1180 (comment)

From the metadata:

public unsafe static extern BOOL InitializeProcThreadAttributeList([Optional][Out][MemorySize(BytesParamIndex = 3)] LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList, [In] uint dwAttributeCount, [Optional][Reserved] uint dwFlags, [In][Out] UIntPtr* lpSize);

[RAIIFree("DeleteProcThreadAttributeList")]
[NativeTypedef]
public struct LPPROC_THREAD_ATTRIBUTE_LIST
{
	public unsafe void* Value;
}

That should be enough to produce an overload like this:

internal static unsafe winmdroot.Foundation.BOOL InitializeProcThreadAttributeList(out DeleteProcThreadAttributeListSafeHandle lpAttributeList, uint dwAttributeCount, uint dwFlags, ref nuint lpSize);

Now, I also see that the out parameter is optional, yet C# doesn't allow for optional out parameters, and the friendly overload couldn't tell whether the caller used out _, so that may also complicate fixing this.

@AArnott AArnott changed the title SafeHandle overload should be generated for functions with out parameters SafeHandle overload should be generated for functions with out parameters May 13, 2024
@waylaa
Copy link

waylaa commented May 14, 2024

Shouldn't every method that includes out parameters generate SafeHandle derived types?

@AArnott
Copy link
Member Author

AArnott commented May 14, 2024

Probably. But I'm not sure I understand the question. Already our SafeHandle overloads return derived types and accept the base type. The out parameter signatures should follow the return type precedent by producing the derived type.

@waylaa
Copy link

waylaa commented May 15, 2024

One method coming from the top of my head that does not contain SafeHandle derivative types is GetBuffer from IAudioCaptureClient, with the method being like this:

unsafe void GetBuffer(byte** ppData, out uint pNumFramesToRead, out uint pdwFlags, [Optional] ulong* pu64DevicePosition, [Optional] ulong* pu64QPCPosition);

and its extension method:

internal static unsafe void GetBuffer(this winmdroot.Media.Audio.IAudioCaptureClient @this, out byte* ppData, out uint pNumFramesToRead, out uint pdwFlags, ulong* pu64DevicePosition, ulong* pu64QPCPosition)
{
    fixed (byte** ppDataLocal = &ppData)
    {
        @this.GetBuffer(ppDataLocal, out pNumFramesToRead, out pdwFlags, pu64DevicePosition, pu64QPCPosition);
    }
}

Couldn't the extension method be like this instead? (pu64DevicePosition and pu64QPCPosition can stay as pointers too)

internal static void GetBuffer(this winmdroot.Media.Audio.IAudioCaptureClient @this, out SafeHandle ppData, out uint pNumFramesToRead, out uint pdwFlags, in ulong pu64DevicePosition, in ulong pu64QPCPosition)

@AArnott
Copy link
Member Author

AArnott commented May 17, 2024

No.

I'm afraid we can't turn byte** parameter types into SafeHandle, as that requires a strongly-typed struct (or typedef in C) so that the metadata can describe what method can be used to release the native resource.

I also don't understand why you would define the last two parameters as in ulong instead of out ulong, considering the direction of data flow as documented for the method.

@waylaa
Copy link

waylaa commented May 17, 2024

I understand. Although it is unfortunate not being able to have SafeHandle there.

I also don't understand why you would define the last two parameters as in ulong instead of out ulong, considering the direction of data flow as documented for the method.

Oops! My bad. I wrote that comment late at evening before going to sleep haha.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants