Missing SID_IDENTIFIER_AUTHORITY constants

[disassembledd]

As an example, SECURITY_NT_AUTHORITY. Referred to in Well-known SIDs list.

[KalleOlaviNiemitalo]

Windows SDK defines:

#define SECURITY_NT_AUTHORITY           {0,0,0,0,0,5}   // ntifs

It is an initializer for the SID_IDENTIFIER_AUTHORITY structure. Would this need a new SidIdentifierAuthorityAttribute type akin to PropertyKeyAttribute, or is there some other way to represent the constant in metadata?

[KalleOlaviNiemitalo]

Other constants whose values are structures but not GUID nor PROPERTYKEY:

• LAP_GIAC_INIT and LAP_LIAC_INIT in shared/bthdef.h; likely not needed, because the LAP_DATA type is not in the SDK, and LAP_GIAC_VALUE and LAP_LIAC_VALUE are in the metadata already
• IN4ADDR_ALLIGMPV3ROUTERSONLINK_INIT etc. in shared/mstcpip.h, for the IN_ADDR type
• EUI48_BROADCAST_INIT in shared/netiodef.h, for the DL_EUI48 type
• DeviceDsmDefinition_Notification etc. in shared/ntddstor.h, for the DEVICE_DSM_DEFINITION type
• IN6ADDR_TEREDOINITIALLINKLOCALADDRESS_INIT etc. in shared/ws2ipdef.h, for the IN6_ADDR type
• CF_CALLBACK_REGISTRATION_END in um/cfapi.h, for the CF_CALLBACK_REGISTRATION type; likely not needed, because CF_CALLBACK_TYPE_NONE is already in the metadata and can be used instead
• DPX_COMPRESSED_FILE_HEADER_XPRESS_HUFF in um/dpx1.idl and um/dpx1.h; not sure whether there is a named type for this
• MI_OPERATION_NULL etc. in um/mi.h, for the MI_Operation etc. types; likely not needed, because the definitions are trivial
• CLUSRES_GET_OPERATION_CONTEXT_FLAGS etc. in um/Resapi.h, apparently for the RESUTIL_PROPERTY_ITEM type
• DeviceDsmDefinition_None etc. in um/winioctl.h, for the DEVICE_DSM_DEFINITION type
• SYSTEM_LUID etc. in um/winnt.h, for the LUID type
• XAUDIO2FX_I3DL2_PRESET_DEFAULT etc. in um/xaudio2fx.h, apparently for the XAUDIO2FX_REVERB_I3DL2_PARAMETERS type

Can there be a generic solution, or do projections have to support each type separately?

Perhaps one could have

[AttributeUsage(AttributeTargets.Class)]
public sealed class ConstInitializer : Attribute
{
}

[ConstInitializer]
[AttributeUsage(AttributeTargets.Field)]
public sealed class SidIdentifierAuthorityAttribute : Attribute
{
    public SidIdentifierAuthorityAttribute(byte[] Value) {}
}

[SidIdentifierAuthority(new byte[] { 0, 0, 0, 0, 0, 5 })]
public static readonly SID_IDENTIFIER_AUTHORITY SECURITY_NT_AUTHORITY;

and let projections recognize just the ConstInitializerAttribute type and then map the parameters of the constructor to the fields of the SID_IDENTIFIER_AUTHORITY type.

[mikebattista]

@kennykerr / @riverar thoughts on this?

[kennykerr]

I really don't like these type-specific attributes, like the existing PropertyKeyAttribute attribute for PROPERTYKEY constants. It's just not generic at all. I'm not sure what the solution is but I'd rather not add more custom attributes if possible. Ideally, we can have some kind of generic and open-ended attribute that can express an initializer list for more than just one type.

[kennykerr]

Or not use an attribute at all but instead use a string constant that language tools can coerce:

.field public static literal valuetype[Windows.Win32.winmd]Windows.Win32.Security.SID_IDENTIFIER_AUTHORITY SECURITY_NT_AUTHORITY = "{0,0,0,0,0,5}"

A similar approach is already being used by constants like TD_WARNING_ICON. The same could be done for constants like PKEY_AudioEndpoint_FormFactor and then we can get rid of the odd PropertyKeyAttribute attribute.

[mikebattista]

@AArnott any thoughts on @kennykerr's recommendation? Are you handling the TD_WARNING_ICON scenario today?

[riverar]

I can't get CsWin32 to generate TD_WARNING_ICON but assuming it did, TaskDialog takes a PWSTR so probably not a great example of TD_WARNING_ICON coercion.

Nevertheless, sounds like a good idea to me. Will need some light docs on how to interpret the list items (e.g. 0,0xA vs 0,10, spaces, alternate ways to represent the list separator , as a list member, etc.)

[AArnott]

I can't get CsWin32 to generate TD_WARNING_ICON

Really? What version are you using, @riverar? It came right up for me and generated this:

internal static readonly unsafe winmdroot.Foundation.PCWSTR TD_WARNING_ICON = (char*)(-1);

This is a simple case because this constant is typed as a typedef struct, so casting is generally sufficient to get it to work.
PROPERTYKEY is significantly more complex because it has 2 fields. Initializing that with just a byte array would require that CsWin32 understand memory layout, the number of bytes in each type, etc. And if those fields were structs in their own right, it would get much worse.
And considering that some structs have sizes and memory layouts that vary with pointer size, a byte array for a constant would seem to be unable to represent all the possibilities.
I'm therefore not bullish on the idea of a general attribute that can be used for all types of constants.

SID_IDENTIFIER_AUTHORITY having only a byte[] seems easy enough to represent via an attribute, if we can't just naturally embed the byte data in metadata, which may be possible.

[kennykerr]

I'm not suggesting copying bits to initialize structs. I don't know of a language where that would work reliably, taking into account alignment and padding. I'm suggesting that when the literal value is a string and the constant type is a struct that the string represent a list of primitive values used to initialize the struct. So if the struct has field int A followed by field int B, the string might be "4,5". Tools would then need to parse this list of values and generate corresponding language-specific code to initialize the struct.

The trouble with the PropertyKeyAttribute approach is that it is not scalable. It does not account for types we don't know about. A third party wants to represent their own types in metadata but cannot because they aren't known in advance by the winmd generator as well as every single language projection.

[AArnott]

Do all the constant structs have only primitive fields? Or do we have to account for structs within structs?

[kennykerr]

We should probably support something like 1, {2, 3}, 4 for structs with fields of structs.

[AArnott]

That should work.

[mikebattista]

I'm not suggesting copying bits to initialize structs. I don't know of a language where that would work reliably, taking into account alignment and padding. I'm suggesting that when the literal value is a string and the constant type is a struct that the string represent a list of primitive values used to initialize the struct. So if the struct has field int A followed by field int B, the string might be "4,5". Tools would then need to parse this list of values and generate corresponding language-specific code to initialize the struct.

Given that we have to define this in C#, what would be the right way to define a constant whose type is a struct but whose value is a string? The obvious options don't compile.

[AArnott]

You can't do it in C# like that. You could use an attribute that takes a string parameter though.

[mikebattista]

Could you clarify then what the spec should be for SECURITY_NT_AUTHORITY?

[AArnott]

I'm hoping @kennykerr can offer an example, as I'm not familiar with this struct/constant.

[kennykerr]

An example of what?

[kennykerr]

Given that we have to define this in C#, what would be the right way to define a constant whose type is a struct but whose value is a string? The obvious options don't compile.

Would an attribute not work?

[Constant("1, {2, 3}, 4")]
public static SOME_STRUCT SOME_CONSTANT;

[riverar]

SECURITY_NT_AUTHORITY example:

[Constant("{ 0, 0, 0, 0, 0, 5 }")]
public static ... SID_IDENTIFIER_AUTHORITY SECURITY_NT_AUTHORITY;

PKEY_Address_Country example:

[Constant("{ { 0xC07B4199, 0xE1DF, 0x4493, 0xB1, 0xE1, 0xDE, 0x59, 0x46, 0xFB, 0x58, 0xF8 }, 100 }")]
public static ... PROPERTYKEY PKEY_Address_Country;

[mikebattista]

Sounds good. Thanks.

[mikebattista]

The build is still not recognizing this for some reason. I don't get any diffs even after a clean build.

I left off at https://github.com/microsoft/win32metadata/tree/mikebattista/structinitializers if someone wants to take a look.

[riverar]

@mikebattista Fixed. 055ba0e

[mikebattista]

Thanks!