GetCurrentProcessToken missing

[timsneath]

also GetCurrentThreadToken and GetCurrentThreadEffectiveToken.

See:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocesstoken
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentthreadeffectivetoken

[sotteson1]

Unfortunately these are inlined functions that use code instead of a DLL import. Maybe we could represent them with attributes:

Header version:

FORCEINLINE
HANDLE
GetCurrentProcessToken (
    VOID
    )
{
    return (HANDLE)(LONG_PTR) -4;
}

Metadata version:

[return: DoNotRelease]
[return: UseConstant(-4)]
public static extern HANDLE GetCurrentProcessToken();

[timsneath]

Yeah, something like that might be a good idea. I don't know how pervasive this kind of inline code is, but it would be helpful to document somehow so that a projection doesn't omit these APIs.

[riverar]

Got asked about this today too. The proposed attributes above look great.

Some additional examples:

FORCEINLINE
HANDLE
GetCurrentProcessToken (
    VOID
    )
{
    return (HANDLE)(LONG_PTR) -4;
}

FORCEINLINE
HANDLE
GetCurrentThreadToken (
    VOID
    )
{
    return (HANDLE)(LONG_PTR) -5;
}

FORCEINLINE
HANDLE
GetCurrentThreadEffectiveToken (
    VOID
    )
{
    return (HANDLE)(LONG_PTR) -6;
}

[riverar]

There are more challenging ones, such as IsBthLEUuidMatch and InitializeThreadpoolEnvironment

FORCEINLINE
BOOLEAN
IsBthLEUuidMatch(
    _In_ BTH_LE_UUID uuid1,
    _In_ BTH_LE_UUID uuid2
    )
{
    BTH_LE_UUID tempLongUuid = {0};

    tempLongUuid.IsShortUuid = FALSE;
    tempLongUuid.Value.LongUuid = BTH_LE_ATT_BLUETOOTH_BASE_GUID;

    if (uuid1.IsShortUuid && uuid2.IsShortUuid) {
        return (uuid1.Value.ShortUuid == uuid2.Value.ShortUuid);
    } else if (!uuid1.IsShortUuid && !uuid2.IsShortUuid) {
        return (0 == memcmp(&uuid1.Value.LongUuid, &uuid2.Value.LongUuid, sizeof(GUID)));
    } else if (uuid1.IsShortUuid) {
        tempLongUuid.Value.LongUuid.Data1 += uuid1.Value.ShortUuid;
        return (0 == memcmp(&tempLongUuid, &uuid2.Value.LongUuid, sizeof(GUID)));
    } else if (uuid2.IsShortUuid) {
        tempLongUuid.Value.LongUuid.Data1 += uuid2.Value.ShortUuid;
        return (0 == memcmp(&uuid1.Value.LongUuid, &tempLongUuid.Value.LongUuid, sizeof(GUID)));
    }

    return FALSE;
}

#ifdef __cplusplus
    }
#endif

FORCEINLINE
VOID
InitializeThreadpoolEnvironment(
    _Out_ PTP_CALLBACK_ENVIRON pcbe
    )
{
    TpInitializeCallbackEnviron(pcbe);
}

[kennykerr]

Please no. 😑

[timsneath]

These are awful. It's a shame the API was written in this way, but we are where we are.

I think it might be simplest for the metadata to simply flag these with some kind of "inline" attribute, and then projections like ours will have to implement them manually in code. But having them in the metadata is important, since otherwise I don't even know that they exist.

[mikebattista]

Please no. 😑

Is the conclusion here to do nothing? Or should we do something about the inlines that return constants? I don't see us doing anything about the more complex inlines.

[kennykerr]

Yes, I'd leave this for now.

[timsneath]

Ideally I'd prefer that we do something here, even if it's relatively lightweight.

After all, these are very much part of the Windows API. It's an unfortunate situation that they are implemented in such a way as to be uncallable without a C compiler being involved, but from a user's perspective, there's no obvious way to even tell from the docs that they are implemented as inline macros (nor should they care).

For simple constants, which are presumably the majority, we could easily support them with an attribute. For more complex ones like IsBthLEUuidMatch, which are hopefully quite rare, we could at least annotate them as inline so that a projection implementer knows that they are going to have to manually recreate them. Right now, I don't even have a mechanism to enumerate them, so I have no way of knowing what I'm missing. And that's a problem!

[mikebattista]

We introduced the Constant attribute with #1337 for use with struct initializers.

Should we also apply it to inline functions that return constants? The suggestion above was UseConstant but if we already have a Constant attribute, we can apply it to functions as well and projections should be able to differentiate the semantics based on the context where it's applied.

[kennykerr]

Sure, if we use the same Constant attribute and limit it to constant-like functions (no parameters) then it should be relatively simple to support.

[mikebattista]

I think I need to provide a DllImport attribute for the manual definition to show up in the winmd. Should I add some placeholder DLL name so parsers aren't broken? Below is what I have showing up in the winmd even though this inline function isn't actually exported from kernel32.dll. I just manually added the attribute so it was emitted.

[DllImport("kernel32.dll", PreserveSig = false)]
[return: Constant("-4")]
public static extern HANDLE GetCurrentProcessToken();

[kennykerr]

If you put the Constant attribute on the method itself rather than the return parameter then parsers can filter them far more efficiently. I don't have a problem with using kernel32 for all such constants. We can just ignore the library name if the Constant attribute is present on the method.

[mikebattista]

How about this?

[DllImport("FORCEINLINE")]
[Constant("-4")]
[return: DoNotRelease]
public static extern HANDLE GetCurrentProcessToken();

[DllImport("FORCEINLINE")]
[Constant("-5")]
[return: DoNotRelease]
public static extern HANDLE GetCurrentThreadToken();

[DllImport("FORCEINLINE")]
[Constant("-6")]
[return: DoNotRelease]
public static extern HANDLE GetCurrentThreadEffectiveToken();

[timsneath]

It doesn't look awfully elegant to have a DllImport that doesn't import a DLL, but I'm with @kennykerr -- in practice it doesn't make much of a difference.

Why are the constants strings and not ints?

[mikebattista]

If you wanted to query all FORCEINLINEs via the metadata, DllImport == "FORCEINLINE" would give you a way assuming we add more that don't just return constants.

The Constant attribute is generic so it can be applied to arbitrary types and structs with arbitrary depths. Projections will need to cast the value to the appropriate type based on the return type of the function.

[riverar]

I'm not a fan of any of this. Feels like a slippery slope situation--why are we doing work for these inline functions and not the billion others?

[mikebattista]

What do you mean? Also you were supportive of this earlier. What changed?

[riverar]

Since my enthusiasic reply in 2021, we've gotten beat up over other inline functions/macros like

• the whole suite of WMI wrappers WMI (MI_xxx) functions - where are they? windows-rs#1572
• windowing macros Add Win32 Macros windows-rs#921
• all the version helper functions C header-only functions and functional macros not getting translated windows-rs#1205
• and a few variant helpers InitPropVariantFromString missing windows-rs#976

These are all virtually part of the Windows API surface too. Right? RIGHT? 🙃

So when evaluating the changes here, I also think of those functions and macros. And I don't think we're going to be able to apply the solution proposed here to those. So now I'm wondering if projection authors really want to add support for yet another attribute given the limited applicability/scope. Would they be better off implementing what they feel their respective community needs/wants manually?

I'm not opposed to try it out though. Metadata is nimble and we're still in pre-release.

One question about the proposed implementation (#436 (comment)): Should implementers assume Constant is a stringified integer? I think this might limit its use for other functions/macros later. Should we perhaps specialize this as IntConstant instead? (Bonus: Can use a native integer over a string.) I don't have any other concerns with the proposed implementation.

[mikebattista]

I'm fine experimenting with these few.

One question about the proposed implementation (#436 (comment)): Should implementers assume Constant is a stringified integer? I think this might limit its use for other functions/macros later. Should we perhaps specialize this as IntConstant instead? (Bonus: Can use a native integer over a string.) I don't have any other concerns with the proposed implementation.

No the type should be deduced from the context (HANDLE in this case). I'd like to avoid type-specific attributes.