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

`EndPaint` take a `ref` second parameter #299

Closed dbremner closed 8 years ago

dbremner commented 8 years ago

Andrew,

This patch fixes #298, which is:

User32.EndPaint(System.IntPtr hWnd, PInvoke.User32.PAINTSTRUCT lpPaint) function should pass its second argument as a ref. I noticed this while updating a project to 0.3.152. The associated pull request works around the problem by changing the Friendly attribute from FriendlyFlag.In to FriendlyFlag.Bidirectional.

Regards, David

AArnott commented 8 years ago

should pass its second argument as a ref

Why should it pass in by ref? The native code claims to treat the PAINTSTRUCT as a const.

I think we should fix it in the Friendly code generation. Otherwise the case will reproduce itself.

I would agree, if I believed the code gen was buggy. But for Friendly(In), the code generator is supposed to not take a ref parameter, since ref implies the callee can change the value of the argument and "In" means that's not expected, so instead it optimizes for easier calling patterns.

AArnott commented 8 years ago

I'm guessing the "should pass its second argument as a ref" is only a perf optimization. And in that case, that's why the primary p/invoke signature we write for many of these methods takes a pointer:

private unsafe void SomeFunc() {
  PAINTSTRUCT ps; // initialized elsewhere.
  User32.EndPaint(hwnd, &ps);
}
vbfox commented 8 years ago

I would agree, if I believed the code gen was buggy. But for Friendly(In), the code generator is supposed to not take a ref parameter, since ref implies the callee can change the value of the argument and "In" means that's not expected, so instead it optimizes for easier calling patterns.

Sorry, I read the PR description without checking the code, the Friendly operator declare helpers, it doesn't add new PInvokes so no problem in fact.

I'm guessing the "should pass its second argument as a ref" is only a perf optimization.

If it's the case it's a small one ( cost of copying structure into the stack) and if it's really needed using a pointer isn't hard.

AArnott commented 8 years ago

Rejecting. We'll follow up in the bug.