[PATCH] msi: Use __stdcall instead of the default __cdecl in IWineMsiRemote.

[Gerald Pfeifer]

Building with clang 7 or new versions of GCC I get the loads of the
following under dlls/msi:

  winemsi_s.c:4014:5: warning: cast between incompatible calling 
      conventions 'cdecl' and 'stdcall'; calls through this pointer 
      may abort at runtime
      [-Wcast-calling-convention]
    (SERVER_ROUTINE)s_remote_ViewClose,
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  ./winemsi.h:69:14: note: consider defining 's_remote_ViewClose' 
      with the 'stdcall' calling convention
  UINT __cdecl s_remote_ViewClose(
               ^
               __attribute__((stdcall)) 

Does the approach below look proper?  If not, how would you address 
this?

Gerald

---
 dlls/msi/msiquery.c  | 12 ++++++------
 dlls/msi/winemsi.idl | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/dlls/msi/msiquery.c b/dlls/msi/msiquery.c
index cf8b2b2b4d..ccdb114226 100644
--- a/dlls/msi/msiquery.c
+++ b/dlls/msi/msiquery.c
@@ -1209,12 +1209,12 @@ MSICONDITION WINAPI MsiDatabaseIsTablePersistentW(
     return r;
 }
 
-UINT __cdecl s_remote_ViewClose(MSIHANDLE view)
+UINT __RPC_API s_remote_ViewClose(MSIHANDLE view)
 {
     return MsiViewClose(view);
 }
 
-UINT __cdecl s_remote_ViewExecute(MSIHANDLE view, struct wire_record *remote_rec)
+UINT __RPC_API s_remote_ViewExecute(MSIHANDLE view, struct wire_record *remote_rec)
 {
     MSIHANDLE rec = 0;
     UINT r;
@@ -1228,7 +1228,7 @@ UINT __cdecl s_remote_ViewExecute(MSIHANDLE view, struct wire_record *remote_rec
     return r;
 }
 
-UINT __cdecl s_remote_ViewFetch(MSIHANDLE view, struct wire_record **rec)
+UINT __RPC_API s_remote_ViewFetch(MSIHANDLE view, struct wire_record **rec)
 {
     MSIHANDLE handle;
     UINT r = MsiViewFetch(view, &handle);
@@ -1239,7 +1239,7 @@ UINT __cdecl s_remote_ViewFetch(MSIHANDLE view, struct wire_record **rec)
     return r;
 }
 
-UINT __cdecl s_remote_ViewGetColumnInfo(MSIHANDLE view, MSICOLINFO info, struct wire_record **rec)
+UINT __RPC_API s_remote_ViewGetColumnInfo(MSIHANDLE view, MSICOLINFO info, struct wire_record **rec)
 {
     MSIHANDLE handle;
     UINT r = MsiViewGetColumnInfo(view, info, &handle);
@@ -1250,7 +1250,7 @@ UINT __cdecl s_remote_ViewGetColumnInfo(MSIHANDLE view, MSICOLINFO info, struct
     return r;
 }
 
-MSIDBERROR __cdecl s_remote_ViewGetError(MSIHANDLE view, LPWSTR *column)
+MSIDBERROR __RPC_API s_remote_ViewGetError(MSIHANDLE view, LPWSTR *column)
 {
     WCHAR empty[1];
     DWORD size = 1;
@@ -1266,7 +1266,7 @@ MSIDBERROR __cdecl s_remote_ViewGetError(MSIHANDLE view, LPWSTR *column)
     return r;
 }
 
-UINT __cdecl s_remote_ViewModify(MSIHANDLE view, MSIMODIFY mode,
+UINT __RPC_API s_remote_ViewModify(MSIHANDLE view, MSIMODIFY mode,
     struct wire_record *remote_rec, struct wire_record **remote_refreshed)
 {
     MSIHANDLE handle = 0;
diff --git a/dlls/msi/winemsi.idl b/dlls/msi/winemsi.idl
index 7888b0ceb4..33a58894ce 100644
--- a/dlls/msi/winemsi.idl
+++ b/dlls/msi/winemsi.idl
@@ -65,12 +65,12 @@ struct wire_record {
 ]
 interface IWineMsiRemote
 {
-    UINT remote_ViewClose( [in] MSIHANDLE view );
-    UINT remote_ViewExecute( [in] MSIHANDLE view, [in, unique] struct wire_record *record );
-    UINT remote_ViewFetch( [in] MSIHANDLE view, [out] struct wire_record **record );
-    UINT remote_ViewGetColumnInfo( [in] MSIHANDLE view, [in] MSICOLINFO info, [out] struct wire_record **record );
-    MSIDBERROR remote_ViewGetError( [in] MSIHANDLE view, [out, string] LPWSTR *column );
-    UINT remote_ViewModify( [in] MSIHANDLE view, [in] MSIMODIFY mode,
+    UINT __stdcall remote_ViewClose( [in] MSIHANDLE view );
+    UINT __stdcall remote_ViewExecute( [in] MSIHANDLE view, [in, unique] struct wire_record *record );
+    UINT __stdcall remote_ViewFetch( [in] MSIHANDLE view, [out] struct wire_record **record );
+    UINT __stdcall remote_ViewGetColumnInfo( [in] MSIHANDLE view, [in] MSICOLINFO info, [out] struct wire_record **record );
+    MSIDBERROR __stdcall remote_ViewGetError( [in] MSIHANDLE view, [out, string] LPWSTR *column );
+    UINT __stdcall remote_ViewModify( [in] MSIHANDLE view, [in] MSIMODIFY mode,
         [in] struct wire_record *record, [out] struct wire_record **refreshed );
 
     MSICONDITION remote_DatabaseIsTablePersistent( [in] MSIHANDLE db, [in] LPCWSTR table );
-- 
2.22.0

[Marvin]

Hi,

While running your changed tests, I think I found new failures.
Being a bot and all I'm not very good at pattern recognition, so I might be
wrong, but could you please double-check?

Full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=55132

Your paranoid android.


=== debian10 (32 bit report) ===

Report errors:
msi:install contains a misplaced todo message for custom

=== debian10 (32 bit Chinese:China report) ===

Report errors:
msi:install contains a misplaced todo message for custom

=== debian10 (32 bit WoW report) ===

Report errors:
msi:install contains a misplaced todo message for custom

=== debian10 (64 bit WoW report) ===

Report errors:
msi:install contains a misplaced todo message for custom

[Zebediah Figura]

On 8/3/19 2:17 PM, Gerald Pfeifer wrote:
> Building with clang 7 or new versions of GCC I get the loads of the
> following under dlls/msi:
>
> winemsi_s.c:4014:5: warning: cast between incompatible calling
>        conventions 'cdecl' and 'stdcall'; calls through this pointer
>        may abort at runtime
>        [-Wcast-calling-convention]
>      (SERVER_ROUTINE)s_remote_ViewClose,
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    ./winemsi.h:69:14: note: consider defining 's_remote_ViewClose'
>        with the 'stdcall' calling convention
>    UINT __cdecl s_remote_ViewClose(
>                 ^
>                 __attribute__((stdcall))
>
> Does the approach below look proper?  If not, how would you address
> this?

So, from my understanding:

* MIDL always generates functions not decorated by calling convention, 
unless one is explicitly specified (I think? I haven't tested this.)

* Microsoft says the default calling convention in C/C++ is cdecl [1], 
so the RPC runtime must presumably expect those functions to be cdecl.

* For stubless mode, I think it doesn't matter which calling convention 
is used (at least between cdecl and stdcall), since the caller can just 
clean up the stack, as we do in call_server_func().

* I don't think the server routine table is used when not using stubless 
mode. (Are we even correct to generate it in that case?)

Personally, I'm inclined to say this warning is not very useful in 
general, and should be ignored. I don't think it's any more an error to 
mismatch calling convention than, say, parameter count or type, when 
casting a function. But of course it's not my decision to make.

[1] https://docs.microsoft.com/en-us/cpp/cpp/cdecl?view=vs-2019