Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 20 June 2011 15:08, Vitaliy Margolen wine-de...@kievinfo.com wrote:
On 06/19/2011 08:15 PM, Austin Lund wrote:
+ ret = wine_server_call( req );
+ shi-Count += reply-handles;
+ len = sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count +
sizeof(ULONG);
+ shi = RtlReAllocateHeap(GetProcessHeap(),
HEAP_ZERO_MEMORY, shi, len);
Please do allocations outside of server call block. And add handling of
failed realloc. Also please double the allocated size, don't reallocate
after each server call.
+ for (i = shi-Count - reply-handles; i shi-Count;
i++) {
+ shi-Handle[i].OwnerPid = reply-pid;
+ }
Please follow file's curly braces style - none for single line blocks, or on
separate line for multi-line blocks.
Thanks for the review. Does this patch address these concerns correctly?
From b4b4310cc28560ea30d1ad6a7961f4c5b443c73a Mon Sep 17 00:00:00 2001
From: Austin Lund austin.l...@gmail.com
Date: Sun, 12 Jun 2011 19:41:18 +1000
Subject: [PATCH 1/2] dlls/ntdll: Add handle count to NtQuerySystemInformation (try 2)
Reply-To: wine-devel wine-devel@winehq.org
---
dlls/ntdll/nt.c | 73 +++
dlls/ntdll/tests/info.c |8 +
2 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index f9302c1..5e7a711 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -1655,18 +1655,79 @@ NTSTATUS WINAPI NtQuerySystemInformation(
break;
case SystemHandleInformation:
{
-SYSTEM_HANDLE_INFORMATION shi;
+SYSTEM_HANDLE_INFORMATION *shi;
+HANDLE hSnap = 0;
+
+FIXME(info_class SYSTEM_HANDLE_INFORMATION partial implementation\n);
+
+SERVER_START_REQ( create_snapshot )
+{
+req-flags = SNAP_PROCESS;
+req-attributes = 0;
+if (!(ret = wine_server_call( req )))
+hSnap = wine_server_ptr_handle( reply-handle );
+}
+SERVER_END_REQ;
+
+len = 128;
+shi = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, len);
+if (shi == NULL)
+return STATUS_NO_MEMORY;
-memset(shi, 0, sizeof(shi));
-len = sizeof(shi);
+for (;;)
+{
+int i, handle_count;
+process_id_t pid;
+
+SERVER_START_REQ( next_process )
+{
+req-handle = wine_server_obj_handle( hSnap );
+req-reset = 0;
+ret = wine_server_call( req );
+handle_count = reply-handles;
+pid = reply-pid;
+}
+SERVER_END_REQ;
+
+if (ret != STATUS_SUCCESS) break;
+
+shi-Count += handle_count;
+while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count = len)
+{
+len = 2*len;
+shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len);
+if (shi == NULL)
+return STATUS_NO_MEMORY;
+
+}
+
+for (i = shi-Count - handle_count; i shi-Count; i++)
+shi-Handle[i].OwnerPid = pid;
+}
+
+len = sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count + sizeof(ULONG);
+shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len);
+if (shi == NULL)
+return STATUS_NO_MEMORY;
+
+if ( ResultLength )
+*ResultLength = len;
if ( Length = len)
{
if (!SystemInformation) ret = STATUS_ACCESS_VIOLATION;
-else memcpy( SystemInformation, shi, len);
+else
+{
+memcpy(SystemInformation, shi, len);
+RtlFreeHeap(GetProcessHeap(), 0, shi);
+shi = NULL;
+ret = STATUS_SUCCESS;
+}
}
-else ret = STATUS_INFO_LENGTH_MISMATCH;
-FIXME(info_class SYSTEM_HANDLE_INFORMATION\n);
+else
+ret = STATUS_INFO_LENGTH_MISMATCH;
+
+if (hSnap) NtClose(hSnap);
}
break;
case SystemCacheInformation:
diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c
index 2d5c94f..f82bdfd 100644
--- a/dlls/ntdll/tests/info.c
+++ b/dlls/ntdll/tests/info.c
@@ -468,7 +468,7 @@ static void test_query_handle(void)
/* Request the needed length : a SystemInformationLength greater than one struct sets ReturnLength */
status = pNtQuerySystemInformation(SystemHandleInformation, shi, SystemInformationLength, ReturnLength);
-todo_wine ok( status == STATUS_INFO_LENGTH_MISMATCH, Expected
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/20/2011 05:28 AM, Austin Lund wrote: Thanks for the review. Does this patch address these concerns correctly? No quite there yet: +SERVER_START_REQ( create_snapshot ) You not checking if this call succeeded or not. +while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count = len) You don't need a loop to calculate new size. +shi = RtlReAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, shi, len); +if (shi == NULL) +return STATUS_NO_MEMORY; You leaking old shi here. Also why do you need to zero allocated memory if you assigning all of it valid values? Vitaliy.
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 20 June 2011 23:52, Vitaliy Margolen wine-de...@kievinfo.com wrote:
+ SERVER_START_REQ( create_snapshot )
You not checking if this call succeeded or not.
I've tried to include all the checks in the new patch.
+ while (sizeof(ULONG) +
sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count = len)
You don't need a loop to calculate new size.
I've done this a different way in the new patch which avoids the reallocs.
You leaking old shi here. Also why do you need to zero allocated memory if
you assigning all of it valid values?
Not all of the values are assigned. Only the handle pid is accessible
with the current wineserver protocol. The remainder of the fields are
best set to zero as this is a sane value for these fields to trigger
exceptions and figure out how/when these values are being accessed
(e.g. null pointers or null handles). Later patches, which I have
already sent, try to fill in more of these fields.
From 8e9631f10f5da50792fd4bc825c6f4b0f8ecac5c Mon Sep 17 00:00:00 2001
From: Austin Lund austin.l...@gmail.com
Date: Sun, 12 Jun 2011 19:41:18 +1000
Subject: [PATCH] dlls/ntdll: Add handle count to NtQuerySystemInformation (try 2)
Reply-To: wine-devel wine-devel@winehq.org
---
dlls/ntdll/nt.c | 109 +++---
dlls/ntdll/tests/info.c |8 +---
2 files changed, 103 insertions(+), 14 deletions(-)
diff --git a/dlls/ntdll/nt.c b/dlls/ntdll/nt.c
index f9302c1..b460aaa 100644
--- a/dlls/ntdll/nt.c
+++ b/dlls/ntdll/nt.c
@@ -1655,18 +1655,111 @@ NTSTATUS WINAPI NtQuerySystemInformation(
break;
case SystemHandleInformation:
{
-SYSTEM_HANDLE_INFORMATION shi;
+SYSTEM_HANDLE_INFORMATION *shi;
+HANDLE hSnap = 0;
+int handle_count, process_handles, i;
+process_id_t pid;
-memset(shi, 0, sizeof(shi));
-len = sizeof(shi);
+FIXME(info_class SYSTEM_HANDLE_INFORMATION partial implementation\n);
-if ( Length = len)
+SERVER_START_REQ( create_snapshot )
{
-if (!SystemInformation) ret = STATUS_ACCESS_VIOLATION;
-else memcpy( SystemInformation, shi, len);
+req-flags = SNAP_PROCESS;
+req-attributes = 0;
+if (!(ret = wine_server_call( req )))
+hSnap = wine_server_ptr_handle( reply-handle );
}
-else ret = STATUS_INFO_LENGTH_MISMATCH;
-FIXME(info_class SYSTEM_HANDLE_INFORMATION\n);
+SERVER_END_REQ;
+
+if (hSnap == 0) return ret;
+
+handle_count = 0;
+
+for (;;)
+{
+SERVER_START_REQ( next_process )
+{
+req-handle = wine_server_obj_handle( hSnap );
+req-reset = 0;
+ret = wine_server_call( req );
+process_handles = reply-handles;
+}
+SERVER_END_REQ;
+
+if (ret == STATUS_SUCCESS)
+handle_count += process_handles;
+else break;
+}
+
+if (ret != STATUS_NO_MORE_FILES)
+{
+NtClose(hSnap);
+return ret;
+}
+
+len = sizeof(ULONG) + handle_count*sizeof(SYSTEM_HANDLE_ENTRY);
+
+if (len Length)
+{
+NtClose(hSnap);
+ret = STATUS_INFO_LENGTH_MISMATCH;
+break;
+}
+
+shi = RtlAllocateHeap(GetProcessHeap(), HEAP_ZERO_MEMORY, len);
+if (shi == NULL)
+{
+NtClose(hSnap);
+ret = STATUS_NO_MEMORY;
+break;
+}
+
+SERVER_START_REQ( next_process )
+{
+req-handle = wine_server_obj_handle( hSnap );
+req-reset = 1;
+ret = wine_server_call( req );
+process_handles = reply-handles;
+pid = reply-pid;
+}
+SERVER_END_REQ;
+
+for (;;)
+{
+if (ret != STATUS_SUCCESS) break;
+
+for (i = 0; i process_handles; i++)
+{
+shi-Handle[shi-Count + i].OwnerPid = pid;
+}
+shi-Count += process_handles;
+
+SERVER_START_REQ( next_process )
+{
+req-handle = wine_server_obj_handle( hSnap );
+req-reset = 0;
+ret = wine_server_call( req );
+process_handles = reply-handles;
+pid = reply-pid;
+}
+SERVER_END_REQ;
+}
+
+NtClose(hSnap);
+
+if (ret != STATUS_NO_MORE_FILES)
+{
+RtlFreeHeap(GetProcessHeap(), 0, shi);
+
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/20/2011 09:36 PM, Austin Lund wrote: On 20 June 2011 23:52, Vitaliy Margolenwine-de...@kievinfo.com wrote: +SERVER_START_REQ( create_snapshot ) You not checking if this call succeeded or not. I've tried to include all the checks in the new patch. +while (sizeof(ULONG) + sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count= len) You don't need a loop to calculate new size. I've done this a different way in the new patch which avoids the reallocs. You leaking old shi here. Also why do you need to zero allocated memory if you assigning all of it valid values? Not all of the values are assigned. Only the handle pid is accessible with the current wineserver protocol. The remainder of the fields are best set to zero as this is a sane value for these fields to trigger exceptions and figure out how/when these values are being accessed (e.g. null pointers or null handles). Later patches, which I have already sent, try to fill in more of these fields. Makes sense. The patch looks good to me. Vitaliy.
Re: [PATCH 1/4] dlls/ntdll: Add handle count to NtQuerySystemInformation
On 06/19/2011 08:15 PM, Austin Lund wrote:
+ret = wine_server_call( req );
+shi-Count += reply-handles;
+len = sizeof(SYSTEM_HANDLE_ENTRY)*shi-Count +
sizeof(ULONG);
+shi = RtlReAllocateHeap(GetProcessHeap(),
HEAP_ZERO_MEMORY, shi, len);
Please do allocations outside of server call block. And add handling of
failed realloc. Also please double the allocated size, don't reallocate
after each server call.
+for (i = shi-Count - reply-handles; i shi-Count; i++)
{
+shi-Handle[i].OwnerPid = reply-pid;
+}
Please follow file's curly braces style - none for single line blocks, or on
separate line for multi-line blocks.
Vitaliy.
