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

void* where it should be IntPtr #434

Closed yves-dolce closed 5 years ago

yves-dolce commented 5 years ago

PostMessage 's parameters, wParam and lParam, are not pointers but additional message-specific information. It feels incorrect to have to mark your project as unsafe, in addition to casting an integral type (e.g. UInt32) to a pointer to be able to call PostMessage's P/Invoke method.

This also applies to the SetWindowLong family methods' dwNewLong parameter and return type.

I have not read/searched the whole source code to find additional methods where void* is used instead of IntPtr.

AArnott commented 5 years ago

In C++ LPARAM and WPARAM are both pointer types (UINT_PTR and ULONG_PTR respectively), so they are either 32-bit or 64-bit depending on the process bitness. So it would be incorrect if we represented them in C# as UInt32 since that's always 32 bits but it must be 64 bits in a 64-bit process.

So pointers are "correct" in the sense that they are the right length and are sometimes the value that folks intend to use. To appeal better to folks who have values that aren't pointers, we also provide an IntPtr overload (as we always do for pointer methods). Folks with an Int32 value can use new IntPtr(int) to construct what they need to call this method.

So I think we're correct here.

yves-dolce commented 5 years ago

As I mentioned in my e-mail exchange with you earlier, and failed again to clarify here, is that they should be changed to IntPtr. Forcing people to generate unsafe code to send numbers with PostMessage is wrong. If they need to send pointers, they can go down that path and cast a void* to an IntPtr. Those parameters are documented as additional data and as I already communicated, a lot of messages do not use any pointer, Also, UINT_PTR and ULONG_PTR are not pointers. They're just _int64 or int depending on the bitness of the OS.

In C++, you still need to reinterpret_casrt<WARAM>(aPointer) if you want to pass the pointer.

I'll do a PR with the related changes and you'll decide if it's worthwhile or not. Thanks for the discussion!

#if defined(_WIN64)
    typedef __int64 INT_PTR, *PINT_PTR;
    typedef unsigned __int64 UINT_PTR, *PUINT_PTR;

    typedef __int64 LONG_PTR, *PLONG_PTR;
    typedef unsigned __int64 ULONG_PTR, *PULONG_PTR;

    #define __int3264   __int64
#else
    typedef _W64 int INT_PTR, *PINT_PTR;
    typedef _W64 unsigned int UINT_PTR, *PUINT_PTR;

    typedef _W64 long LONG_PTR, *PLONG_PTR;
    typedef _W64 unsigned long ULONG_PTR, *PULONG_PTR;

    #define __int3264   __int32
#endif
AArnott commented 5 years ago

Forcing people to generate unsafe code to send numbers with PostMessage is wrong.

I suspect you're only looking at our source code to make this assertion. We are not forcing anyone to use unsafe code. An IntPtr overload is offered too, so folks can choose whether to use the IntPtr overload or the void* overload.

image

yves-dolce commented 5 years ago

Thanks Andrew.

Yes I am. I saw that in src\User32\PublicAPI.Shipped.txt but was wondering. How/when/where is this generated as it's nowhere to be found in the source code?

Using that method marked as unsafe, you still need to allow unsafe build though, don't you?

AArnott commented 5 years ago

How/when/where is this generated as it's nowhere to be found in the source code?

Code generation happens during the build. We inspect our own public API and then add to it;

https://github.com/AArnott/pinvoke/blob/3d19670578f9940781e09a005954337c0ca21844/src/CodeGeneration/OfferFriendlyOverloadsGenerator.cs#L19-L20

Using that method marked as unsafe, you still need to allow unsafe build though, don't you?

No, unsafe on a method signature does not require that callers also allow unsafe code. It's easy to think that, I agree, but we tested it to make sure.

yves-dolce commented 5 years ago

That CodeGeneration's source code is pretty interesting. Thanks for your patience Andrew.