How to correctly call P/Invoke methods in a class library?

Question

I have multiple projects in a Visual Studio 2015 solution. Several of these projects do P/Invokes like:

 [DllImport("IpHlpApi.dll")]
        [return: MarshalAs(UnmanagedType.U4)]
        public static extern int GetIpNetTable(IntPtr pIpNetTable, [MarshalAs(UnmanagedType.U4)]
        ref int pdwSize, bool bOrder);

So I moved all my P/Invokes to a separate class library and defined the single class as:

namespace NativeMethods
{
    [
    SuppressUnmanagedCodeSecurityAttribute(),
    ComVisible(false)
    ]

    public static class SafeNativeMethods
    {
        [DllImport("kernel32.dll", CharSet = CharSet.Auto, ExactSpelling = true)]
        public static extern int GetTickCount();

        // Declare the GetIpNetTable function.
        [DllImport("IpHlpApi.dll")]
        [return: MarshalAs(UnmanagedType.U4)]
        public static extern int GetIpNetTable(IntPtr pIpNetTable, [MarshalAs(UnmanagedType.U4)]
        ref int pdwSize, bool bOrder);
    }
}

From the other projects, this code is called as:

 int result = SafeNativeMethods.GetIpNetTable(IntPtr.Zero, ref bytesNeeded, false);

All compiles without error or warning.

Now running FxCop on the code gives the warning:

Warning CA1401 Change the accessibility of P/Invoke 'SafeNativeMethods.GetIpNetTable(IntPtr, ref int, bool)' so that it is no longer visible from outside its assembly.

Ok. Changing the accessibility to internal as:

[DllImport("IpHlpApi.dll")]
[return: MarshalAs(UnmanagedType.U4)]
internal static extern int GetIpNetTable(IntPtr pIpNetTable, [MarshalAs(UnmanagedType.U4)]
ref int pdwSize, bool bOrder);

Now causes the hard error of:

Error CS0122 'SafeNativeMethods.GetIpNetTable(IntPtr, ref int, bool)' is inaccessible due to its protection level

So how can I make this work without error or warning?

Thanks in advance for any help as I've been going in circles for hours!

FxCop is complaining because it's considered bad style to expose native methods. To refactor your assemblies this way, and still hide the PInvokes from the outside world, you have to make them internal, and add InternalsVisibleTo attributes to that assembly for each of the other assemblies you want to have access. — Zastai, Mar 05 '16 at 19:16
Or, of course, instead of having a separate assembly, just have a single source with the P/Invokes (as internal) and include that in every project that needs them (e.g. as an svn external, or referring to a shared location). — Zastai, Mar 05 '16 at 19:19
@Zastai I'm new to these concepts :( An example would be most helpful. Thanks. I especially like your second idea, but don't know how to do it. — Alan Wayne, Mar 05 '16 at 19:49

score 17 · Accepted Answer · edited May 23 '17 at 11:44

It is a sure thing that you will agree with statement that PInvoke methods are not the most pleasant things to call from C# code.

They are:

Not so strongly typed - often riddled with IntPtr and Byte[] parameters.
Error prone - it is easy to pass some incorrectly initialized parameter, like a buffer with wrong length, or some struct with field not initialized to that struct's size...
Obviously don't throw exceptions if something goes wrong - it is their consumer's responsibility to check the return code or Marshal.GetLastError() it. And more often then not, someone forgets to do it, leading to hard-to-track bugs.

In comparison with these issues FxCop warning is but a meager style checker peeve.

So, what can you do? Deal with those three issues and FxCop will go by itself.

These are the things I recommend you to do:

Do not expose any API directly. It is important for complex functions, but applying it for any function will actually deal with your primary FxCop issue:

public static class ErrorHandling
{
    // It is private so no FxCop should trouble you
    [DllImport(DllNames.Kernel32)]
    private static extern void SetLastErrorNative(UInt32 dwErrCode);

    public static void SetLastError(Int32 errorCode)
    {
        SetLastErrorNative(unchecked((UInt32)errorCode));
    }
}

Don't use IntPtr if you can use some safe handle.

Don't just return Boolean or (U)Int32 from wrapper methods - check for return type inside the wrapper method and throw the exception if required. If you want to use a method in exception-less manner, then provide Try-like version that will clearly denote that it is a no-exception method.

public static class Window
{
    public class WindowHandle : SafeHandle ...

    [return: MarshalAs(UnmanagedType.Bool)]
    [DllImport(DllNames.User32, EntryPoint="SetForegroundWindow")]
    private static extern Boolean TrySetForegroundWindowNative(WindowHandle hWnd);

    // It is clear for everyone, that the return value should be checked.
    public static Boolean TrySetForegroundWindow(WindowHandle hWnd)
    {
        if (hWnd == null)
            throw new ArgumentNullException(paramName: nameof(hWnd));

        return TrySetForegroundWindowNative(hWnd);
    }

    public static void SetForegroundWindow(WindowHandle hWnd)
    {
        if (hWnd == null)
            throw new ArgumentNullException(paramName: nameof(hWnd));

        var isSet = TrySetForegroundWindow(hWnd);
        if (!isSet)
            throw new InvalidOperationException(
                String.Format(
                    "Failed to set foreground window {0}", 
                    hWnd.DangerousGetHandle());
    }
}

Don't use IntPtr or Byte[] if you can use normal structs passed by ref/out. You may say that it is obvious, but in many cases where a strongly typed struct can be passed I have seen IntPtr being used instead. Don't use out parameters in your public-facing methods. In most cases it is unnecessary - you can just return the value.

public static class SystemInformation
{
    public struct SYSTEM_INFO { ... };

    [DllImport(DllNames.Kernel32, EntryPoint="GetSystemInfo")]
    private static extern GetSystemInfoNative(out SYSTEM_INFO lpSystemInfo);

    public static SYSTEM_INFO GetSystemInfo()
    {
        SYSTEM_INFO info;
        GetSystemInfoNative(out info);
        return info;
    }
}

Enums. WinApi uses a lot of enum values as parameters or return values. Being a C style enum they are actually passed(returned) as simple integers. But C# enums are actually nothing more than integers too, so assuming that you have set proper underlying type, you will have much easier to use methods.

Bit/Byte twiddling - whenever you see that getting some values or checking their correctness requires some masks, then you can be sure that it can be better handled with custom wrappers. Sometimes it is handled with FieldOffset, sometimes a bit of actual bit twiddling should be done, but in any case it will be done only in one place, providing simple and handy object model:

public static class KeyBoardInput
{
    public enum VmKeyScanState : byte
    {
        SHIFT = 1,
        CTRL = 2, ...
    }           

    public enum VirtualKeyCode : byte
    {
        ...
    }

    [StructLayout(LayoutKind.Explicit)]
    public struct VmKeyScanResult
    {
        [FieldOffset(0)]
        private VirtualKeyCode _virtualKey;
        [FieldOffset(1)]
        private VmKeyScanState _scanState;

        public VirtualKeyCode VirtualKey
        {
            get {return this._virtualKey}
        }
        public VmKeyScanState ScanState
        {
            get {return this._scanState;}
        }

        public Boolean IsFailure
        {
            get
            {
                return 
                    (this._scanState == 0xFF) &&
                    (this._virtualKey == 0xFF)
            }                   
        }
    }


    [DllImport(DllNames.User32, CharSet=CharSet.Unicode, EntryPoint="VmKeyScan")]
    private static extern VmKeyScanResult VmKeyScanNative(Char ch);

    public static VmKeyScanResult TryVmKeyScan(Char ch)
    {
        return VmKeyScanNative(ch);
    }

    public static VmKeyScanResult VmKeyScan(Char ch)
    {
        var result = VmKeyScanNative(ch);   
        if (result.IsFailure)
            throw new InvalidOperationException(
                String.Format(
                    "Failed to VmKeyScan the '{0}' char",
                    ch));
        return result;
    }
}

P.S.: And do not forget about correct function signatures (bitness and other issues), marshaling of types, layout attributes and char set (also, not forgetting to use DllImport(... SetLastError = true) is of utmost importance). http://www.pinvoke.net/ may often help, but it doesn't always provide the best signature to use.

P.S.1: And I recommend you to organize your NativeMethods not into one class , because it will quickly become one huge unmanageable pile of quite different methods, but instead to group them into separate classes (I actually use one partial root class and nested classes for each functional area - a bit more of tedious typing, but much better context and Intellisense). For class names I just use the same classification MSDN uses to group API functions. Like for GetSystemInfo it is "System Information Functions"

So, if you apply all those advises, you will be able to create a robust, easy to use native wrapper library that hides all the unnecessary complexities and error-prone constructs, but that will look very familiar to anyone who knows the original API.

You added the suffix `Native` to the p/invoke calls, will .net strip that off or did you forget to set the name in the `DllImport.EntryPoint` call? — Scott Chamberlain, Mar 05 '16 at 20:58

How to correctly call P/Invoke methods in a class library?

1 Answers1

Linked