dotnet / pinvoke

A library containing all P/Invoke code so you don't have to import it every time. Maintained and updated to support the latest Windows OS.
MIT License
2.12k stars 222 forks source link

Question about "void *" #326

Closed Lakritzator closed 7 years ago

Lakritzator commented 7 years ago

Reading this:

Provide a slightly higher than lowest level API for P/Invoke signatures. For example, instead of IntPtr parameters and uint flags, you'll see SafeHandle-derived types as parameters and flags enum types. API documentation will be provided via XML doc comments for easy reading with Intellisense, along with links to the describing pages on MSDN or elsewhere as applicable.

Made really sense to me, so today I started to see if it's feasible to use your projects instead of maintaining my own code. What threw me off in the first place is that the first call I was replacing with an implementation of yours already gave me something I wasn't expecting:

I had User32.GetClipboardData defined as

        [DllImport("user32", SetLastError = true)]
        private static extern IntPtr GetClipboardData(uint format);

Where the return value is IntPtr.

The pinvoke repository on the other hand uses "void *" (and int instead of the documented uint):

    [DllImport("User32", SetLastError = true)]
    public static void* GetClipboardData(int uFormat);

Which forces me to use unsafe code, which I in general don't like.

Now my question is: is the usage of pointer is a design decision or if this is just a part of the API which is not finished?

Lakritzator commented 7 years ago

With the next call I tried to replace, Kernel32.GlobalUnlock, it seems to be the same as this also expects a "void *" instead of IntPtr.

So somehow you have a design which seems to "force" this, which is okay but I would like to understand the reason for this.

Lakritzator commented 7 years ago

I actually found some information in the CONTRIBUTING.md which wasn't the first place where I would look (as I wasn't contributing yet from my point of view) but this is an issue with github itself... Sorry about that.

It says

Use native pointers instead of IntPtr. We have automatic code generation in place during the build to create IntPtr overloads of these methods. The only known exception where IntPtr is OK to use directly is when the value isn't actually a pointer that should ever be dereferenced (e.g. a handle that is only ever returned back to the library that produced it).

So I looked again, and I finally found the overloads, they are post-fixed with _IntPtr. It somehow doesn't feel natural, I rather had the IntPtr version without funny post-fix, but it works...

vbfox commented 7 years ago

It somehow doesn't feel natural, I rather had the IntPtr version without funny post-fix, but it works...

While we support non-unsafe code (With IntPtr overloads for example) the library intent is to map to the native API closely and here as GetClipboardData returns a pointer that should really be used as a pointer (To a sequence of TCHAR instances terminated by \0 if it's a CF_TEXT buffer for example) we don't hide that fact in the default method exposed.

Which forces me to use unsafe code, which I in general don't like.

Can I ask you why ?

GetClipboardData is intristically unsafe as it provide you with a pointer with a lifetime you should manually handle and doesn't even provide you with a size information.

Once you're using such a low-level & unsafe API in your code, using the unsafe keyword or not doesn't change anything, you already are in very unsafe territory.

AArnott commented 7 years ago

Which forces me to use unsafe code, which I in general don't like.

I had a stigma against unsafe code as well -- till I worked on PInvoke and realized that writing "safe" code with IntPtr is heaps harder than just using native pointers. All the casting and the Marshal calls are super hard to get right, avoid memory leaks, etc. But unsafe C# lets you use stackalloc and other keywords that make it quite natural and easier to do the right thing.

We support IntPtr for VB callers. You can certainly use it for C# if you want, but I would suggest you try it both ways for one or more of your callers before finalizing your decision.

Also just FYI, we don't require the oddly named overload most of the time. Usually it's the same method name. But when the only pointer that appears is on the return type, we have to give one of the methods another name because overloads can't vary by return type only. And as @vbfox said, we picked native pointers as the default because it's actually easier to work with.

AArnott commented 7 years ago

I've added docs to the README to help alleviate this confusion in the future. Thanks for raising this.