Constants like `MSIDBOPEN_PATCHFILE`, et al, should be PCWSTR-typed like others are.

[mjr4077au]

Actual behavior

The raw P/Invoke for MsiOpenDatabase() accepts szPersist as a PCWSTR which matches the header, but the constants that need to be sent through are just integers.

Expected behavior

That constants be typed to the value of the underlying P/Invoke. Other P/Invokes like TaskDialog(), which takes PCWSTR constants has the constants like TD_INFORMATION_ICON properly typed as PCWSTR.

Repro steps

1. NativeMethods.txt content:

MSIDBOPEN_PATCHFILE

2. NativeMethods.json content (if present): N/A

3. Any of your own code that should be shared? N/A

Context

• CsWin32 version: 0.3.257
• Win32Metadata version (if explicitly set by project): N/A
• Target Framework: net472
• LangVersion (if explicitly set by project): N/A

[mjr4077au]

Apologies, I just came to realise everything but MSIDBOPEN_PATCHFILE is a PCWSTR, so it's just this value that's not typed right.

EDIT: So, it's typed right according to the SDK (#define MSIDBOPEN_PATCHFILE 32/sizeof(*MSIDBOPEN_READONLY)), however it is inconsistent with the other values and isn't directly consumable.

It's also not directly usable as the value needs to be 32 at all times these days: https://devblogs.microsoft.com/setup/opening-patch-files-when-compiled-for-unicode/.

While this makes it not necessarily a CsWin32 issue, it does mean that the constant is not directly usable and is also incorrect even if it was typed properly. I'll leave it open for someone here to say whether this is just how it is or whether something better can/should be done.

[riverar]

// msiquery.h
#define MSIDBOPEN_READONLY     (LPCTSTR)0  // database open read-only, no persistent changes
#define MSIDBOPEN_TRANSACT     (LPCTSTR)1  // database read/write in transaction mode
#define MSIDBOPEN_DIRECT       (LPCTSTR)2  // database direct read/write without transaction
#define MSIDBOPEN_CREATE       (LPCTSTR)3  // create new database, transact mode read/write
#define MSIDBOPEN_CREATEDIRECT (LPCTSTR)4  // create new database, direct mode read/write
#define MSIDBOPEN_PATCHFILE    32/sizeof(*MSIDBOPEN_READONLY) // add flag to indicate patch file

Looks like an unfortunate header bug. Probably makes more sense to push this upstream rather than try to monkey patch win32metadata and misrepresent the constant.

[mjr4077au]

Regrettably, I believe the header's been this way for some time; at least 20 years if not more. The value of 32/sizeof(*MSIDBOPEN_READONLY) is also incorrect as that's the required value for ANSI, back when people were making MSIs for Windows 9x. 32 is the correct value one needs to use today for unicode.

Does all of this put it in the "too hard basket"? Not sure what the best direction is here. It's clearly a bad setup, however it's been like this for ~1/4 century.

[riverar]

Doesn't appear this was available before Windows XP. And note that it appears the only supported scenario is MSIDBOPEN_READONLY + MSIDBOPEN_PATCHFILE which correctly results in (LPCTSTR)32U due to (LPCTSTR)0 + 16 pointer arithmetic. (It appears Heath Stewart meant well but the blog post seems fundamentally wrong.)

[mjr4077au]

His blog post was absolutely the one I was referencing. His blog post makes some sense though as LPCTSTR is only 1 byte in size if UNICODE isn't defined (Windows 9x, etc). To me the header feels like a bad setup by someone who wasn't thinking about unicode environments when they were on the tools.