Add NTDll.RtlGetVersion #498
Conversation
|
Thanks, @vatsan-madhavan ! |
src/NTDll.Tests/NTDllFacts.cs
Outdated
| { | ||
| var versionInfo = Kernel32.OSVERSIONINFO.Create(); | ||
|
|
||
| Assert.Equal(NTSTATUS.Code.STATUS_SUCCESS, RtlGetVersion((Kernel32.OSVERSIONINFOEX*)&versionInfo).Value); |
There was a problem hiding this comment.
I wonder whether this is a case where helpers could come in handy. For e.g., in NtDll.Helpers.cs (see Kernel32.Helpers.cs for example), perhaps add a public overload RtlGetVersion(OSVERSIONINFO*) and "hide" this type of "reinterpret_cast" type casting in there (and let the users/callers of this API get a clean experience) ?
Another related thought - if you do add a helper, then would it make sense to use RtlGetVersion(OSVERSIONINFO) as the DllImport method, and RtlGetVersion(OSVERSIONINFOEX) as the helper? This would make the code for the helper itself a bit cleaner (the cast would read like (OSVERSIONINFO*)&osVersionInfoEx, which would guarantee that the pointer is within valid bounds and that this fact is statically verifiable) and would also follow the pattern in the OS itself (GetVersion and RtlGetVersion are defined in terms of LPOSVERSIONINFO, and when the dwOSVersionInfoSize indicates that the payload is actually an LPOSVERSIONINFOEX then additional fields are filled in and returned.)
Kernel32+PRODUCT_SUITE: Add VER_SUITE_MULTIUSERTS Unit test: report on invalid value
…es are explicitly defined.
|
I think I addressed all comments for this PR, let me know if there's anything else you need from my side! |
They required 32-bits to represent but were defined in a 16-bit enum.
| /// <summary> | ||
| /// AppServer mode is enabled. | ||
| /// </summary> | ||
| VER_SUITE_MULTIUSERTS = unchecked((short)0x00020000), | ||
|
|
||
| /// <summary> | ||
| /// Windows NT server is installed. | ||
| /// </summary> | ||
| VER_SERVER_NT = unchecked((short)0x80000000), | ||
|
|
||
| /// <summary> | ||
| /// Windows NT Workstation is installed. | ||
| /// </summary> | ||
| VER_WORKSTATION_NT = unchecked((short)0x40000000), |
There was a problem hiding this comment.
These enum values are all 0 because you're truncating the numbers you're providing to just 16 bits, and the last 16 bits are all zero. Clearly these could never be used in this enum which apparently is intentionally sized at 16 bits to fit the WORD size used in the methods that take this enum.
I've removed these new values.
No description provided.