Re: [PATCH v4 1/5] ntdll: Return BIOS info from NtQuerySystemInformation on Linux
On Wed, Jun 13, 2018 at 09:41:15PM -0600, Alex Henrie wrote:
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/nt.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index dc0ce04f42..72ae381813 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -66,6 +66,50 @@
WINE_DEFAULT_DEBUG_CHANNEL(ntdll);
+#include "pshpack1.h" + +typedef enum _SYSTEM_FIRMWARE_TABLE_ACTION +{ + SystemFirmwareTable_Enumerate = 0, + SystemFirmwareTable_Get = 1 +} SYSTEM_FIRMWARE_TABLE_ACTION; + +typedef struct _SYSTEM_FIRMWARE_TABLE_INFORMATION +{ + ULONG ProviderSignature; + SYSTEM_FIRMWARE_TABLE_ACTION Action; + ULONG TableID; + ULONG TableBufferLength; + UCHAR TableBuffer[1]; +} SYSTEM_FIRMWARE_TABLE_INFORMATION;
These two at least should be in winternl.h It fails to build on macOS nt.c:2454:27: error: label at end of compound statement: expected statement case RSMB: I suspect you may want to move the main body of the implementation into a helper function, to avoid adding even more bloat to the main switch. Also, some tests would be nice, both for the ntdll (free feel to just tidy up mine) and kernel32 functions. Note, that these went to wine-patches rather than wine-devel. Huw.
On Thu, Jun 14, 2018 at 2:07 AM Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Jun 13, 2018 at 09:41:15PM -0600, Alex Henrie wrote:
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/nt.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index dc0ce04f42..72ae381813 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -66,6 +66,50 @@
WINE_DEFAULT_DEBUG_CHANNEL(ntdll);
+#include "pshpack1.h" + +typedef enum _SYSTEM_FIRMWARE_TABLE_ACTION +{ + SystemFirmwareTable_Enumerate = 0, + SystemFirmwareTable_Get = 1 +} SYSTEM_FIRMWARE_TABLE_ACTION; + +typedef struct _SYSTEM_FIRMWARE_TABLE_INFORMATION +{ + ULONG ProviderSignature; + SYSTEM_FIRMWARE_TABLE_ACTION Action; + ULONG TableID; + ULONG TableBufferLength; + UCHAR TableBuffer[1]; +} SYSTEM_FIRMWARE_TABLE_INFORMATION;
These two at least should be in winternl.h
Are you sure? They're not in the native winternl.h.
It fails to build on macOS nt.c:2454:27: error: label at end of compound statement: expected statement case RSMB:
I suspect you may want to move the main body of the implementation into a helper function, to avoid adding even more bloat to the main switch.
Thanks for catching the Mac OS build error. While I'm fixing that I'll see what I can do to organize the code better.
Also, some tests would be nice, both for the ntdll (free feel to just tidy up mine) and kernel32 functions.
Your test just checks that NtQuerySystemInformation(SystemFirmwareTableInformation, ...) returns STATUS_SUCCESS, which doesn't seem very useful. The returned data itself is dependent on the BIOS and could be very different on different computers.
Note, that these went to wine-patches rather than wine-devel.
Yes, my mistake, old habits die hard... Thanks for the feedback. -Alex
On Thu, Jun 14, 2018 at 07:26:54AM -0600, Alex Henrie wrote:
On Thu, Jun 14, 2018 at 2:07 AM Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Jun 13, 2018 at 09:41:15PM -0600, Alex Henrie wrote:
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/nt.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index dc0ce04f42..72ae381813 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -66,6 +66,50 @@
WINE_DEFAULT_DEBUG_CHANNEL(ntdll);
+#include "pshpack1.h" + +typedef enum _SYSTEM_FIRMWARE_TABLE_ACTION +{ + SystemFirmwareTable_Enumerate = 0, + SystemFirmwareTable_Get = 1 +} SYSTEM_FIRMWARE_TABLE_ACTION; + +typedef struct _SYSTEM_FIRMWARE_TABLE_INFORMATION +{ + ULONG ProviderSignature; + SYSTEM_FIRMWARE_TABLE_ACTION Action; + ULONG TableID; + ULONG TableBufferLength; + UCHAR TableBuffer[1]; +} SYSTEM_FIRMWARE_TABLE_INFORMATION;
These two at least should be in winternl.h
Are you sure? They're not in the native winternl.h.
I thought we had other 'struct SYSTEM_' definitions in there that aren't in the native SDK.
Also, some tests would be nice, both for the ntdll (free feel to just tidy up mine) and kernel32 functions.
Your test just checks that NtQuerySystemInformation(SystemFirmwareTableInformation, ...) returns STATUS_SUCCESS, which doesn't seem very useful. The returned data itself is dependent on the BIOS and could be very different on different computers.
Well it must be useful because you claimed it returned STATUS_NOT_IMPLEMENTED before I wrote it ;-) You obviously can't test the data, but testing the lengths (i.e. the returned length and buffer length) is also interesting. In principle the kernel32 tests could compare the data returned from a call to the ntdll function with the data returned from the kernel32 function. Huw.
On Thu, Jun 14, 2018 at 7:52 AM Huw Davies <huw(a)codeweavers.com> wrote:
On Thu, Jun 14, 2018 at 07:26:54AM -0600, Alex Henrie wrote:
On Thu, Jun 14, 2018 at 2:07 AM Huw Davies <huw(a)codeweavers.com> wrote:
On Wed, Jun 13, 2018 at 09:41:15PM -0600, Alex Henrie wrote:
Signed-off-by: Alex Henrie <alexhenrie24(a)gmail.com> --- dlls/ntdll/nt.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c index dc0ce04f42..72ae381813 100644 --- a/dlls/ntdll/nt.c +++ b/dlls/ntdll/nt.c @@ -66,6 +66,50 @@
WINE_DEFAULT_DEBUG_CHANNEL(ntdll);
+#include "pshpack1.h" + +typedef enum _SYSTEM_FIRMWARE_TABLE_ACTION +{ + SystemFirmwareTable_Enumerate = 0, + SystemFirmwareTable_Get = 1 +} SYSTEM_FIRMWARE_TABLE_ACTION; + +typedef struct _SYSTEM_FIRMWARE_TABLE_INFORMATION +{ + ULONG ProviderSignature; + SYSTEM_FIRMWARE_TABLE_ACTION Action; + ULONG TableID; + ULONG TableBufferLength; + UCHAR TableBuffer[1]; +} SYSTEM_FIRMWARE_TABLE_INFORMATION;
These two at least should be in winternl.h
Are you sure? They're not in the native winternl.h.
I thought we had other 'struct SYSTEM_' definitions in there that aren't in the native SDK.
Here are the SYSTEM structs I found in Wine's winternl.h but not in the Windows SDK: SYSTEM_CACHE_INFORMATION SYSTEM_CONFIGURATION_INFO SYSTEM_CPU_INFORMATION SYSTEM_DRIVER_INFORMATION SYSTEM_HANDLE_ENTRY SYSTEM_HANDLE_INFORMATION SYSTEM_KERNEL_DEBUGGER_INFORMATION SYSTEM_MODULE SYSTEM_MODULE_INFORMATION SYSTEM_TIME_ADJUSTMENT This suggests that we're not worrying about Winelib-compiled programs including winternl.h and then defining those structs on their own (which would cause a redefinition error during compilation). If that's really not something to be concerned about then sure, I can throw SYSTEM_FIRMWARE_TABLE_ACTION and SYSTEM_FIRMWARE_TABLE_INFORMATION into winternl.h too.
Also, some tests would be nice, both for the ntdll (free feel to just tidy up mine) and kernel32 functions.
Your test just checks that NtQuerySystemInformation(SystemFirmwareTableInformation, ...) returns STATUS_SUCCESS, which doesn't seem very useful. The returned data itself is dependent on the BIOS and could be very different on different computers.
Well it must be useful because you claimed it returned STATUS_NOT_IMPLEMENTED before I wrote it ;-) You obviously can't test the data, but testing the lengths (i.e. the returned length and buffer length) is also interesting.
In principle the kernel32 tests could compare the data returned from a call to the ntdll function with the data returned from the kernel32 function.
OK, that sounds reasonable. -Alex
participants (2)
-
Alex Henrie -
Huw Davies