search

microsoft / win32metadata

Tooling to generate metadata for Win32 APIs in the Windows SDK.
Other
1.31k stars 114 forks source link

RM_PROCESS_INFO::TSSessionId is unsigned but RM_INVALID_TS_SESSION is signed #1927

Open KalleOlaviNiemitalo opened 1 month ago

KalleOlaviNiemitalo commented 1 month ago

RM_INVALID_TS_SESSION is intended to be compared to RM_PROCESS_INFO::TSSessionId but they have different types; one is signed and the other is unsigned. That then requires an unchecked cast when these are compared in C#.

https://github.com/microsoft/win32metadata/blob/c2b7ea95b5ae31b76db8f45bbe353b48a58a65f7/generation/WinSDK/RecompiledIdlHeaders/um/RestartManager.h#L31-L32https://github.com/microsoft/win32metadata/blob/c2b7ea95b5ae31b76db8f45bbe353b48a58a65f7/generation/WinSDK/RecompiledIdlHeaders/um/RestartManager.h#L99-L100

Can the same type be used for both? I guess it would have to be the unsigned type, for consistency with WTS_CURRENT_SESSION, WTSQuerySessionInformationW, and others.

KalleOlaviNiemitalo commented 1 month ago

Similar considerations may apply to RM_INVALID_PROCESS vs. RM_UNIQUE_PROCESS::dwProcessId, but I haven't found any formal documentation on the intended use of RM_INVALID_PROCESS.

https://github.com/microsoft/win32metadata/blob/c2b7ea95b5ae31b76db8f45bbe353b48a58a65f7/generation/WinSDK/RecompiledIdlHeaders/um/RestartManager.h#L33-L34https://github.com/microsoft/win32metadata/blob/c2b7ea95b5ae31b76db8f45bbe353b48a58a65f7/generation/WinSDK/RecompiledIdlHeaders/um/RestartManager.h#L88

mikebattista commented 1 month ago

I wouldn't change the type of the struct fields. Should the constant values be changed to reflect any implicit conversions that C is doing?