microsoft / CsWin32

A source generator to add a user-defined set of Win32 P/Invoke methods and supporting types to a C# project.
MIT License
2.11k stars 89 forks source link

Friendly overload should expose [Out] PWSTR in more friendly way #614

Open palenshus opened 2 years ago

palenshus commented 2 years ago

Today accessing such a method requires something like:

Span<char> text = stackalloc char[cTxtLen]; // value gotten from GetWindowTextLength
unsafe
{
    fixed (char* pText = text)
    {
        PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, pText, cTxtLen );
    }
}

Ideally this would instead allow something like:

Span<char> text = new char[cTxtLen]; // value gotten from GetWindowTextLength
PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, text, cTxtLen);

Or:

var text = new StringBuilder(cTxtLen); // value gotten from GetWindowTextLength
PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, text, cTxtLen);

Or even:

var text = new StringBuilder(cTxtLen); // value gotten from GetWindowTextLength
PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, text);

Since presumably the wrapper can see what the buffer size is without the user having to provide that again. Thoughts?

AArnott commented 2 years ago

I think your first approach of accepting a Span<char> is the way we'll go. The length parameter will be interesting. If it's strictly an input then we can probably infer it from the span's length. But sometimes its an output, or an in/out parameter. We may not optimize all those paths right away, but there are some great opportunities there.

craigktreasure commented 2 years ago

Hitting the same on GetCurrentPackageFullName. Would love to be able to pass a stack allocated Span<char>.

elachlan commented 2 years ago

Could we template it so that the friendly method doesn't even need the length? It can all happen internally and just return a span.

AArnott commented 2 years ago

@elachlan, yes. We already do remove the length parameter in friendly overloads in some cases. See here:

https://github.com/microsoft/CsWin32/blob/22521d1e0d87660f8caad4f2f2733026e5633968/src/Microsoft.Windows.CsWin32/Generator.cs#L4701-L4707

riverar commented 1 year ago

+1, GetModuleFileName is similarly awkward to use.

BennyBread commented 10 months ago

For Example GetKeyboardLayoutName can be calles using StringBuilder when the code is written by hand:

[DllImport("user32.dll")] private static extern bool GetKeyboardLayoutName(StringBuilder pwszKlid);

When I use PInvoke.GetKeyboardLayoutName I am forced to use PWSTR and an this " very beautiful" (sarcasm) fixed unsave, char pointer code....

There are many functions which can use StringBuilder.

Wouldn't it be nice to have stringbuilder everywhere where it's possible?

The old pinvoke.net site listed many of them. Unfortunately the site is gone..

elachlan commented 10 months ago

@BennyBread using StringBuilder in a PInvoke is a perf issue, see: https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1838

These calls can avoid the heap (creating a class) by using span (stack allocation). You want lowest level most performant, but with the ease of access to transform it to a string.

Rand-Random commented 9 months ago

Can someone enlighten a noob, how I could have possible known that this code is necessary?

Span<char> text = stackalloc char[cTxtLen]; // value gotten from GetWindowTextLength
unsafe
{
    fixed (char* pText = text)
    {
        PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, pText, cTxtLen );
    }
}

I have tried the following, going off with the signature of the method

var pwstr = new PWSTR();
PInvoke.GetWindowText((Windows.Win32.Foundation.HWND)window, pwstr, cTxtLen );
var title = pwstr.ToString();

But, as you may know and I simply run into it, this code always returns an empty string.

There for I am all in favor of this issue.

RFBomb commented 3 months ago

I was just writing up my own issue regarding this, as I ran into it with GetFinalPathNameByHandle

I adapted the above to this:

Span<char> text = new Span<char>(new char[(int)Win32.PInvoke.MAX_PATH]);
uint result;
fixed(char* builder = text)
{
    result = Win32.PInvoke.GetFinalPathNameByHandle(fileHandle, builder, Win32.PInvoke.MAX_PATH, GETFINALPATHNAMEBYHANDLE_FLAGS.FILE_NAME_NORMALIZED);
}

which wound up working for me. But it was not at all clear when I was writing it what I was doing wrong, until I found this thread. I think having some overloads that accept the span would be ideal here.