dotnet / pinvoke Goto Github PK

When applying the FriendlyAttribute to the second parameter of Process32First:

[DllImport(nameof(Kernel32), SetLastError = true, CharSet = CharSet.Unicode)]
public static extern unsafe bool Process32First(
    SafeObjectHandle hSnapshot,
    [Friendly(FriendlyFlags.Bidirectional)] PROCESSENTRY32* lppe);

The code gen'd friendly overload is typed as ref PROCESSENTRY32* instead of ref PROCESSENTRY32.

The following image shows the problem in the PInvoke.Kernel32 package.

It would be nice to exclude CodeGenerationAttributes.Net40.dll from the package.

We already have ThrowOnFailure calling Marshal.ThrowExceptionForHR but don't have any equivalent to Marshal.GetExceptionForHR.

Foo Function()
{
    Foo result;
    HResult hr = SomePInvoke(out result);
    if (hr.Succeeded)
    {
        return result;
    }
    else
    {
        if (hr == /*Some value*/)
        {
            throw MyError();
        }
        throw hr.GetException();
    }
}

It's often possible to do without but it's sometimes preferable to have explicit throw for control-flow and it ease porting code using the Mashal class

BCrypt's KEY_LENGTHS struct has a bug in its enumerator that would cause it to spin forever if Increment is zero, which it is for DES

The values aren't specified so C# auto-generate 0, 1, ... but as pointed in #26 by @jmelosegui it's incorrect according to msdn

I don't think it's a good idea to mix helpers and non-helpers in the same class name. Others have no clear indication of when something is a helper method, and typically beforehand they are either looking for the "barebone low-level" native API approach, or a high-level assisted approach to solving a problem.

I say we change the name of the helper classes to something else or use a specific naming convention for helpers, so that people can clearly tell the difference.

Ideally, we could even put the helpers in a separate NuGet package. I never want high-level stuff and I think it pollutes my API if they are mixed.

Imagine 3 packages:

• PInvoke.BCrypt
• PInvoke.BCrypt.Mockables (this one could also be included as discussed other places)
• PInvoke.BCrypt.Helpers

I think that's the right approach and the convention that we see elsewhere.

It turns out that building is a lot slower now. Task Manager shows powershell spending a lot of time during the build. I don't yet know whether it's one of the ps1 scripts with a bad algorithm or (more likely) just spawning powershell.exe itself taking a long time. But we should see what we can do to reduce the time.

When a P/Invoke method has an out byte* parameter type, the generated code produces a compiler error:

CS1510  A ref or out argument must be an assignable variable    

I think we need to do something similar to #101 but that is causing a problem with Visual Studio (at least on my PC).

See NuGet/Home/#1831 for more details.

See gitter.im discussion

For example, in the below p/invoke method, we want to generate both an overload that takes IntPtr for all native pointer parameters, and another overload where the byte* parameter type is treated specially by replacing it with byte[].

It seems useful to generate byte[] overloads both with native pointers for the other parameters as well as of course the IntPtr variety. Meaning that we generate 3 overloads instead of 1.

So this hand-written method:

public static unsafe extern SECURITY_STATUS NCryptDecrypt(
    SafeKeyHandle hKey,
    byte* pbInput,
    int cbInput,
    void* pPaddingInfo,
    byte* pbOutput,
    int cbOutput,
    out int pcbResult,
    NCryptEncryptFlags dwFlags);

Would generate these three overloads: (the first is already generated, the rest are proposed)

public static unsafe extern SECURITY_STATUS NCryptDecrypt(
    SafeKeyHandle hKey,
    IntPtr pbInput,
    int cbInput,
    IntPtr pPaddingInfo,
    IntPtr pbOutput,
    int cbOutput,
    out int pcbResult,
    NCryptEncryptFlags dwFlags);

public static unsafe extern SECURITY_STATUS NCryptDecrypt(
    SafeKeyHandle hKey,
    byte[] pbInput,
    int cbInput,
    IntPtr pPaddingInfo,
    byte[] pbOutput,
    int cbOutput,
    out int pcbResult,
    NCryptEncryptFlags dwFlags);

public static unsafe extern SECURITY_STATUS NCryptDecrypt(
    SafeKeyHandle hKey,
    byte[] pbInput,
    int cbInput,
    void* pPaddingInfo,
    byte[] pbOutput,
    int cbOutput,
    out int pcbResult,
    NCryptEncryptFlags dwFlags);

Hey @AArnott!

Glad you had time when I didn't and took this idea further than ever. I also like where it's going, and I will be monitoring it closely over the upcoming weeks. Maybe I'll even contribute once in a while!

Keep up the great effort.

Having Bcrypt.cs be the name of the file with the product code as well as the test code is confusing.

#76 should be done first. See that issue for the rationale.

For now we'll use only a few pointers when deemed necessary and hand-craft IntPtr alternatives, but what would really be interesting is to only have to write the pointer versions and have the IntPtr overload auto-generated.

Code generation will also be needed for #55 so both issues would need the same infrastructure.

I installed the Windows 10 standalone SDK, and I can't find my header files for PInvoke based operations. Do you know where they are?

To ship a stable NuGet package, our API must be stable. That means we need to agree on a fully-spec'd proposal for naming conventions and make sure they are consistently applied in the actual code.

Some of our SafeHandle have it but others don't. I think that we should be consistent and always provide this constructor that is really useful when interacting with other code (Like passing handles created by WPF or winforms to PInvoke methods)

/// <summary>
/// Initializes a new instance of the <see cref="SafeThemeHandle"/> class.
/// </summary>
/// <param name="preexistingHandle">An object that represents the pre-existing handle to use.</param>
/// <param name="ownsHandle">
///     <see langword="true" /> to have the native handle released when this safe handle is disposed or finalized;
///     <see langword="false" /> otherwise.
/// </param>
public SafeThemeHandle(IntPtr preexistingHandle, bool ownsHandle = true)
    : base(IntPtr.Zero, ownsHandle)
{
    this.SetHandle(preexistingHandle);
}

(The info should also be added to CONTRIBUTING in this case)

See #197 for a PR adding 2 such constructors.

Per @vbfox's suggestion

I'm thinking of a package in the same spirit as Microsoft.CodeAnalysis or xunit that could be named PInvoke

One consideration is that for now the library contains only pinvoke for windows dlls (delivered with the operating system) so the package would be named more exactly PInvoke.Windows.

The main advantage for the package I see is for easy prototyping (For example in LINQPad) where searching beforehand for all necessary packages might be more complex than just referencing a meta package.

What do you think ?

We should macro-ize the licenseURL so that it points at the very version of the LICENSE.txt file that accompanied the built source code, instead of always pointing at the tip of 'master'.

See AArnott/PCLCrypto@617f00a for an example of changes to the nuspec/project file to enable this.

But NuProj, which we're using, does not provide a way to specify these macros. So I've opened nuproj/nuproj#183 to track that.

As an increasing number of PInvoke assemblies need to allow unsafe blocks, and we're p/invoking into native code, after all, it makes sense to just allow it for all projects.

Let's add it to EnlistmentInfo.props, and remove the explicit property definition from the individual projects that already have them added.

<AllowUnsafeBlocks>true</AllowUnsafeBlocks>

See @hmemcpy's feedback on gitter

IntPtr parameters are harder to deal with than native pointer types (in C# anyway), and they express less in terms of type safety than pointers can.

We should consider changing our policy and code to use pointers instead of IntPtr.

One possibility is that we maintain two overloads of applicable P/Invoke methods: one that takes IntPtr and one that takes pointers, so that folks can call without using unsafe code. I don't have much sympathy for any anxiety over writing unsafe code since it's actually simpler, and partial trust is irrelevant for consumers of this library anyway. So the only reason to keep IntPtr around is for languages that don't support pointers (e.g. Powershell).

• AdvApi32
• BCrypt
• DbgHelp
• Gdi32
• Hid
• ImageHlp
• Kernel32
• NCrypt
• Psapi
• SetupApi
• User32

With update 1 RTW, the build breaks. It seems Nuproj.common in Windows.Core's project.json file no longer propagates. This may be a bug in Update 1 RTW, and we should follow up on that as well.

C:\Users\andarno\git\pinvoke\src\Kernel32.Desktop\Kernel32.Desktop.csproj : error MSB4057: The target "_NuProjGetProjectClosure" does not exist in the project.
C:\Users\andarno\git\pinvoke\src\Kernel32\Kernel32.csproj : error MSB4057: The target "_NuProjGetProjectClosure" does not exist in the project.

The NTStatus enum is incomplete. We should fill it out, complete with summary xml doc comments for each one, as provided in the documentation. The set of statuses is so large that we should complete this by writing a script that can parse the table out of the MSDN docs (or a header file somewhere).

MSDN NTStatus documentation

We use class instead of struct on some types because it makes it possible to pass in null for optional pointer parameters. But this has some negative side effects:

1. The user may expect the type is a struct and be surprised that a lack of ref or out still produces changes from native code into the managed code.
2. A memory allocation is required to initialize one.
3. The optional vs. required trait that drives use of class is not a characteristic of the type, but rather of a particular use of it in a method. So if some methods require it and other methods make it optional, the type cannot be both.

Now that we're more pointer friendly, I wonder if we should just use structs. We can use native pointers to structs instead of ref or classes for optional parameters.

Structs currently masquerading as classes:

• SP_DEVINFO_DATA
• NullableGuid
• NullableUInt32
• NullablePipeMode
• PROCESSENTRY32
• SECURITY_ATTRIBUTES

The property lpDescription should be decorated as [MarshalAs(UnmanagedType.LPWStr)] to be consistent with methods defined as CharSet.Unicode

This work represents both the changes to remove the IntPtr fields from existing structs, replacing them with native pointer types, as well as adding the IntPtr property accessors for those new fields.

• Code generator
• BCrypt
• DbgHelp
• Kernel32

After this change #87 I think should be good to provide the portable project with the helper methods.

Hi there.

Often when working with PInvoke code, you have a function like this:

public void DoSomething() { MyApi.SomePInvokeCall(); SomeOtherCodeThatIsRelevant(); }

But then you can't unit test DoSomething because it uses a PInvoke API. In other words, it would be an integration test, and if the API in question does something nasty, you would never want it to run.

What if we (from every static class of APIs) backed them up by an interface, which was then injectable through IOC?

Yes, it would require an instance of the class (maybe we could add a Singleton pattern if some people wouldn't ever want to instantiate each API). But it would make huge amounts of code easier to test, and take PInvoke a step further.

So instead of:

public static class SomeExampleApi
    {
        [StructLayout(LayoutKind.Sequential)]
        public struct BITMAP
        {
            public int Type;
            public int Width;
            public int Height;
            public int WidthBytes;
            public ushort Planes;
            public ushort BitsPixel;
            public IntPtr Bits;
        }

        [DllImport("gdi32", CharSet = CharSet.Auto)]
        public static extern int GetObject(
            IntPtr hgdiobj,
            int cbBuffer,
            out BITMAP lpvObject
            );
}

We would have:

private class SomeExampleApi : ISomeExampleApi
    {
        [StructLayout(LayoutKind.Sequential)]
        public struct BITMAP
        {
            public int Type;
            public int Width;
            public int Height;
            public int WidthBytes;
            public ushort Planes;
            public ushort BitsPixel;
            public IntPtr Bits;
        }

        public int GetObject(
            IntPtr hgdiobj,
            int cbBuffer,
            out BITMAP lpvObject
            ) {
               return GetObjectApi(hgdiobj, cbBuffer, out lpvObject);
        }

        [DllImport("gdi32", EntryPoint = "GetObject", CharSet = CharSet.Auto)]
        private static extern int GetObjectApi(
            IntPtr hgdiobj,
            int cbBuffer,
            out BITMAP lpvObject
            );
}

And then the ISomeExampleApi would look like:

public interface ISomeExampleApi {
     public int GetObject(
            IntPtr hgdiobj,
            int cbBuffer,
            out BITMAP lpvObject
            );
}

That way, I could inject an ISomeExampleApi whenever I wanted to use it, and then when testing, fake it out with my favorite mocking framework, and still test other functionality.

I'm getting the following exception on my machine:

Error   MSB4018 The "ReadPdbSourceFiles" task failed unexpectedly.
System.IO.FileNotFoundException: Retrieving the COM class factory for component with CLSID {3BFCEA48-620F-4B6B-81F7-B9AF75454C7D} failed due to the following error: 8007007e The specified module could not be found. (Exception from HRESULT: 0x8007007E).
   at System.RuntimeTypeHandle.CreateInstance(RuntimeType type, Boolean publicOnly, Boolean noCheck, Boolean& canBeCached, RuntimeMethodHandleInternal& ctor, Boolean& bNeedSecurityCheck)
   at System.RuntimeType.CreateInstanceSlow(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
   at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly, Boolean skipCheckThis, Boolean fillCache, StackCrawlMark& stackMark)
   at System.Activator.CreateInstance(Type type, Boolean nonPublic)
   at System.Activator.CreateInstance(Type type)
   at NuProj.Tasks.PdbReader.ReadSourceFiles(String pdbPath) in C:\git\nuproj\src\NuProj.Tasks\ReadPdbSourceFiles.cs:line 36
   at NuProj.Tasks.ReadPdbSourceFiles.Execute() in C:\git\nuproj\src\NuProj.Tasks\ReadPdbSourceFiles.cs:line 25
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext() MSCorEE.NuGet   C:\Users\hmemcpy\.nuget\packages\NuProj\0.10.27-beta-g2363d7cd98\tools\NuProj.targets   963 

This happens for every nuproj project. Perhaps this is more of a question/issue for the nuproj repo...

Windows 10, VS2015. Any ideas?

There is a library out there called SharpDX. They allow full DirectX 11 support for C# through PInvoke-ish APIs, but the cool thing is that they are all auto-generated and auto-converted to C#.

Could we instead of what we are doing now get inspiration from them, and make a tool which auto-generates all C# code?

It would also make it a lot easier to maintain.

Kernel32's ReadFile has a void* parameter, but using byte* may be more appropriate and result in code gen helper methods for byte[] being generated.

We should stop sharing source for AlgorithmIdentifiers between NCrypt and BCrypt so that folks who use both in a using static way aren't befuddled in their efforts.

See discussion here: #34 (diff)

I expect some folks will more readily consume the work of this project if they can merge the P/Invokes they use into their assembly rather than shipping all the unused ones as well in a separate assembly.

A NuGet package that uses ILMerge as a build step may allow this process to be very easy for consumers to opt into.

The AdvApi32+ServiceTrigger struct has a member that points to a SERVICE_TRIGGER_SPECIFIC_DATA_ITEM struct, but that struct isn't defined. Once we define it, we'll probably want to change pDataItems to be typed as SERVICE_TRIGGER_SPECIFIC_DATA_ITEM[] so that would be an API breaking change.

Can we fit it in before the API goes stable?

We should make our type derive from Win32Exception as defined by the BCL when it's available (on desktop profile) so that folks can 'catch' these exceptions by its base type.

When this change is in place, NTStatus.ThrowOnError() and other related methods should use it instead of throwing COMException.

Is there any pattern to how we write the PInvoke classes? It doesn't seem like there are any of the libraries that are 100% done yet.

Wouldn't it be a good idea to finish entire DLL imports for whole DLL files at a time, for instance finishing user32.dll first before working on others?

To use this library, I need to know that it has everything I need before downloading it. As long as it isn't 100% done, I am not using it (due to the fear of something missing).

Also, I think there are some areas that are more used than others (for instance shell32.dll, kernel32.dll and user32.dll). Could we start with these?

From this AppVeyor build

PInvoke.Win32 -> C:\projects\pinvoke\bin\Release\Packages\PInvoke.Win32.0.1.286-beta-ge68a656187.nupkg
 420  C:\projects\pinvoke\tools\Create-PInvokeTxtFile.ps1 : Cannot bind argument to 
 421  parameter 'AssemblyPath' because it is an empty string.
 422  At line:1 char:68
 423  + & { & 'C:\projects\pinvoke\src\..\tools\Create-PInvokeTxtFile.ps1' '' }
 424  +                                                                    ~~
 425      + CategoryInfo          : InvalidData: (:) [Create-PInvokeTxtFile.ps1], Pa 
 426     rameterBindingValidationException
 427      + FullyQualifiedErrorId : ParameterArgumentValidationErrorEmptyStringNotAl 
 428     lowed,Create-PInvokeTxtFile.ps1
 429   
 430  CodeGeneration.Debugging -> C:\projects\pinvoke\src\CodeGeneration.Debugging\bin\Release\CodeGeneration.Debugging.exe

I wonder if this is because the specialized build scripts aren't prepared to skip processing of the code generation projects.

Audio Switcher by @davkean is a cool project that use quite a bit of P/Invoke and it would be nice to have enough API available that we can send it a PR to use our library.

What we miss as APIs :

• Kernel32\EnumResourceNames
• Kernel32\FindResource
• Kernel32\LoadResource
• Kernel32\LockResource
• Kernel32\SizeofResource
• Kernel32\LoadLibraryEx
• User32\LookupIconIdFromDirectory
• User32\LookupIconIdFromDirectoryEx
• User32\GetDC
• User32\ReleaseDC
• Gdi32\GetDeviceCaps
• Uxtheme\GetThemeMargins

Except for the last one it's on dlls we already have in the codebase and the license of the project is also MIT so we can reuse directly the declaration after adaptation.

We also need a few types :

• GRPICONDIR
• GRPICONDIRENTRY
• ICONDIR
• ICONDIRENTRY
  Even if the lib is never used in Audio Switcher in the end we will have gained a few more functions.

We should consider defining enums or structs to assist with all three of these, and then use them instead of int32 in all our interop code. Also (where possible) offer conversion between them and exception throwing routines for them where the portable profile lacks them.

Here are some good resources for getting started:

• http://stackoverflow.com/questions/25566234/how-to-convert-specific-ntstatus-value-to-the-hresult
• http://jpassing.com/2007/08/20/error-codes-win32-vs-hresult-vs-ntstatus/
• https://msdn.microsoft.com/en-us/library/windows/desktop/ms680600(v=vs.85).aspx

We should display badges in HTML rendered versions of our README.md file indicating how many exported functions of each native binary we have P/Invoke methods for. For example, if Kernel32.dll has 533 exported functions and we have p/invoke methods for 12 of them, the badge might look like this:

Benefits

1. Set expectations appropriately with consumers of the packages as to the likelihood that what they care about is already in there.
2. I think it may help motivate contributors to drive the numbers higher as it will make a visible different on our landing page and make the project more compelling.

Design

There are two halves to the analysis we need to do:

1. We need to create a maintainable list of documented, exported methods from a native binary.
2. We need to create a maintainable list of [DllImport] methods in the managed P/Invoke library for that native binary.

The process should involve extracting the right set of method names from the native binary and maintaining them in a list. Then assembling a similar list from the managed binary. These lists should be created such that method names will match for each list if they exist in both.

Native binary analysis

The following command will print out all exported functions (i.e. candidates for P/Invokes)

dumpbin /exports LIBNAME.dll

Once we have this list, we'll need to de-duplicate for ansi vs. Unicode versions. We should recognize when a method ends with A or W, and if so, check for another method with the same name except the alternate final letter. When we see this, we report the root method name (without the A or W suffix). If we don't recognize such a pattern, we report the full method name.

The resulting list should be checked into source control as a text file with exactly one function name per line. Its content should be sorted alphabetically. The filename should be LIBNAME.exports.txt. While we should have a script that can produce this file, as its results would only vary with the version of the native DLL, and as we want a consistent target to aim for across dev and build machines, we should check in the result and only update it when a new version of the native library is released. This also lets us (potentially) hand edit the 'target' file to remove functions that are undocumented and thus not intended to be P/Invoked into.

The build should copy this LIBNAME.exports.txt file as a build output.

Managed P/Invoke assembly analysis

Use reflection on the managed P/Invoke library to find all static methods (independent of their visibility) that have [DllImport] attributes on them. When the DllImportAttribute.Entrypoint property is set on the attribute, report that as the native function name. Otherwise report the method name itself.

This list should be generated at build time and saved to a text file named LIBNAME.pinvokes.txt and dropped as a build output. Each line should contain exactly one of the names reported as described above and these lines should be sorted alphabetically.

Producing shields/badges

The LIBNAME.exports.txt and LIBNAME.pinvokes.txt files should be found side-by-side as build outputs. Perhaps in a dedicated directory such as bin\debug\exports. These files should be captured by AppVeyor as artifacts.

A simple build-time script will compare the LIBNAME.exports.txt and LIBNAME.pinvokes.txt file to count how many lines each contains to formulate a string with the form X/Y where X is the number of lines in the LIBNAME.pinvokes.txt file and Y is the number of lines in the LIBNAME.exports.txt file. This string can then be injected into this URL to replace PLACEHOLDER:

https://img.shields.io/badge/P%2FInvokes-PLACEHOLDER-orange.svg

We might even change the color based on how X and Y compare to each other. For example:

Condition	Color	Example
X = Y	Green
else X > Y * 8/10	YellowGreen
else X > Y * 4/10	Orange
else X > 0	Red
else	LightGrey

These badges should act as hyperlinks to a document with a table that compares the LIBNAME.pinvokes.txt and LIBNAME.exports.txt file so users can quickly discover whether the functions they need are already available.

Unsolved

1. Is executing dumpbin /exports and parsing the output really the best way to go or is there a programmatic approach we can take instead?
2. Somehow we need to get this X/Y string or URL into an such that it appears in renderings of README.md.
3. We could also produce a Markdown table listing all the exported functions and a column showing whether we have a P/Invoke method for it. But where/how would this Markdown file be hosted/viewed?

WinRT (on PC) is fine, but on wpa81 in particular, Kernel32's FormatMessage method seems to be unreachable, as shown in the emulator screenshot below.
I've sent an email to a Microsoft DL asking for more info since the MSDN doc for FormatMessage suggests this should work.

See #40 for some background.

The current tests use files but we have no way to control whenever the IO will really be asynchronous or not, we should use pipes instead where we can ensure that a synchronous answer is impossible.

It would also allow to correctly test IO Cancelation support.

I implies adding support for CreatePipe or CreateNamedPipe.

Our existing libraries should be checked and updated to properly support modern platforms by applying the new API Sets guidance.

The following projects need to be reviewed:

• AdvApi32
• BCrypt
• DbgHelp
• Gdi32
• Hid
• ImageHlp
• Kernel32
• NCrypt
• Psapi
• SetupApi
• User32