Hi Daniel,
thanks for working on this. I have a couple of remarks, see below.
On 29.04.2016 03:53, Daniel Lehman wrote:
Signed-off-by: Daniel Lehman dlehman25@gmail.com
dlls/ntdll/sync.c | 38 +++++++++++++---- dlls/ntdll/tests/om.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++ server/mutex.c | 16 ++++++++ server/protocol.def | 10 +++++ 4 files changed, 167 insertions(+), 9 deletions(-)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c index e87e672..576593f 100644 --- a/dlls/ntdll/sync.c +++ b/dlls/ntdll/sync.c @@ -520,15 +520,35 @@ NTSTATUS WINAPI NtReleaseMutant( IN HANDLE handle, OUT PLONG prev_count OPTIONAL
NtQueryMutant [NTDLL.@]ZwQueryMutant [NTDLL.@]*/ -NTSTATUS WINAPI NtQueryMutant(IN HANDLE handle,
IN MUTANT_INFORMATION_CLASS MutantInformationClass,OUT PVOID MutantInformation,IN ULONG MutantInformationLength,OUT PULONG ResultLength OPTIONAL )-{
- FIXME("(%p %u %p %u %p): stub!\n",
handle, MutantInformationClass, MutantInformation, MutantInformationLength, ResultLength);
Please convert the FIXME to a TRACE instead of removing it.
- return STATUS_NOT_IMPLEMENTED;
+NTSTATUS WINAPI NtQueryMutant( HANDLE handle, MUTANT_INFORMATION_CLASS class,
void *info, ULONG len, ULONG *ret_len )+{
- NTSTATUS ret;
- MUTANT_BASIC_INFORMATION *out = info;
- if (class != MutantBasicInformation)
- {
FIXME("(%p, %d, %d) Unknown class\n",handle, class, len);return STATUS_INVALID_INFO_CLASS;- }
- if (len != sizeof(MUTANT_BASIC_INFORMATION)) return STATUS_INFO_LENGTH_MISMATCH;
- SERVER_START_REQ( query_mutex )
- {
req->handle = wine_server_obj_handle( handle );if (!(ret = wine_server_call( req ))){out->CurrentCount = 1 - reply->count;out->OwnedByCaller = reply->owned;out->AbandonedState = reply->abandoned;if (ret_len) *ret_len = sizeof(MUTANT_BASIC_INFORMATION);}- }
- SERVER_END_REQ;
- return ret;
}
/* diff --git a/dlls/ntdll/tests/om.c b/dlls/ntdll/tests/om.c index c05f31d..ee850d3 100644 --- a/dlls/ntdll/tests/om.c +++ b/dlls/ntdll/tests/om.c @@ -43,6 +43,8 @@ static NTSTATUS (WINAPI *pNtCreateMailslotFile)( PHANDLE, ACCESS_MASK, POBJECT_A ULONG, ULONG, ULONG, PLARGE_INTEGER ); static NTSTATUS (WINAPI *pNtCreateMutant)( PHANDLE, ACCESS_MASK, const POBJECT_ATTRIBUTES, BOOLEAN ); static NTSTATUS (WINAPI *pNtOpenMutant) ( PHANDLE, ACCESS_MASK, const POBJECT_ATTRIBUTES ); +static NTSTATUS (WINAPI *pNtQueryMutant) ( HANDLE, MUTANT_INFORMATION_CLASS, PVOID, ULONG, PULONG ); +static NTSTATUS (WINAPI *pNtReleaseMutant)( HANDLE, PLONG ); static NTSTATUS (WINAPI *pNtCreateSemaphore)( PHANDLE, ACCESS_MASK,const POBJECT_ATTRIBUTES,LONG,LONG ); static NTSTATUS (WINAPI *pNtOpenSemaphore)( PHANDLE, ACCESS_MASK, const POBJECT_ATTRIBUTES ); static NTSTATUS (WINAPI *pNtCreateTimer) ( PHANDLE, ACCESS_MASK, const POBJECT_ATTRIBUTES, TIMER_TYPE ); @@ -1865,6 +1867,113 @@ static void test_null_device(void) CloseHandle(ov.hEvent); }
+static DWORD WINAPI mutant_thread( void *arg ) +{
- MUTANT_BASIC_INFORMATION info;
- NTSTATUS status;
- HANDLE mutant;
- mutant = arg;
- status = WaitForSingleObject( mutant, 1000 );
- ok( status == STATUS_SUCCESS, "WaitForSingleObject failed %08x\n", status );
WaitForSingleObject does not return STATUS_* values. This error also appears a couple more times below.
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == 0, "NtQueryMutant failed, expected 0, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == TRUE, "NtQueryMutant failed, expected TRUE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.AbandonedState );
Its a matter of taste, but I am not sure if it makes sense to duplicate the "NtQueryMutant failed". Better print the variable which was tested, for example "expected CurrentCount == 0, got %d\n", and so on.
- /* abandon mutant */
- return 0;
+}
+static void test_mutant(void) +{
- static const WCHAR name[] = {'\','B','a','s','e','N','a','m','e','d','O','b','j','e','c','t','s',
'\\','t','e','s','t','_','m','u','t','a','n','t',0};- MUTANT_BASIC_INFORMATION info;
- OBJECT_ATTRIBUTES attr;
- UNICODE_STRING str;
- NTSTATUS status;
- HANDLE mutant;
- HANDLE thread;
- ULONG len;
- LONG prev;
- pRtlInitUnicodeString(&str, name);
- InitializeObjectAttributes(&attr, &str, 0, 0, NULL);
- status = pNtCreateMutant(&mutant, GENERIC_ALL, &attr, TRUE);
- ok( status == STATUS_SUCCESS, "Failed to create Mutant(%08x)\n", status );
- /* bogus */
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, 0, NULL);
- ok( status == STATUS_INFO_LENGTH_MISMATCH,
"Failed to NtQueryMutant, expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status );- status = pNtQueryMutant(mutant, 0x42, &info, sizeof(info), NULL);
- ok( status == STATUS_INVALID_INFO_CLASS,
"Failed to NtQueryMutant, expected STATUS_INVALID_INFO_CLASS, got %08x\n", status );- status = pNtQueryMutant((HANDLE)0xdeadbeef, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_INVALID_HANDLE,
"Failed to NtQueryMutant, expected STATUS_INVALID_HANDLE, got %08x\n", status );
Not sure if you have seen it, but your patch causes new test failures, see: http://newtestbot.winehq.org/JobDetails.pl?Key=22713
- /* new */
- len = -1;
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), &len);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == 0, "NtQueryMutant failed, expected 0, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == TRUE, "NtQueryMutant failed, expected TRUE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.AbandonedState );
- ok( len == sizeof(info), "NtQueryMutant failed, expected %u, got %u\n", (DWORD)sizeof(info), len );
Its better to avoid printing sizeof() values, only print the "len" value here.
There is a whitespace issue in the line above.
- status = WaitForSingleObject( mutant, 1000 );
- ok( status == STATUS_SUCCESS, "WaitForSingleObject failed %08x\n", status );
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == -1, "NtQueryMutant failed, expected -1, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == TRUE, "NtQueryMutant failed, expected TRUE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.AbandonedState );
- prev = 0xdeadbeef;
- status = pNtReleaseMutant(mutant, &prev);
- ok( status == STATUS_SUCCESS, "NtQueryRelease failed %08x\n", status );
- todo_wine ok( prev == -1, "NtQueryRelease failed, expected -1, got %d\n", prev );
- prev = 0xdeadbeef;
- status = pNtReleaseMutant(mutant, &prev);
- ok( status == STATUS_SUCCESS, "NtQueryRelease failed %08x\n", status );
- todo_wine ok( prev == 0, "NtQueryRelease failed, expected 0, got %d\n", prev );
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == 1, "NtQueryMutant failed, expected 1, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.AbandonedState );
- /* abandoned */
- thread = CreateThread( NULL, 0, mutant_thread, mutant, 0, NULL );
- status = WaitForSingleObject( thread, 1000 );
- ok( status == 0, "WaitForSingleObject failed %08x\n", status );
- NtClose( thread );
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == 1, "NtQueryMutant failed, expected 0, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == TRUE, "NtQueryMutant failed, expected TRUE, got %d\n", info.AbandonedState );
- status = WaitForSingleObject( mutant, 1000 );
- ok( status == STATUS_ABANDONED_WAIT_0, "WaitForSingleObject failed %08x\n", status );
- status = pNtQueryMutant(mutant, MutantBasicInformation, &info, sizeof(info), NULL);
- ok( status == STATUS_SUCCESS, "NtQueryMutant failed %08x\n", status );
- ok( info.CurrentCount == 0, "NtQueryMutant failed, expected 0, got %d\n", info.CurrentCount );
- ok( info.OwnedByCaller == TRUE, "NtQueryMutant failed, expected TRUE, got %d\n", info.OwnedByCaller );
- ok( info.AbandonedState == FALSE, "NtQueryMutant failed, expected FALSE, got %d\n", info.AbandonedState );
- NtClose( mutant );
+}
START_TEST(om) { HMODULE hntdll = GetModuleHandleA("ntdll.dll"); @@ -1892,6 +2001,8 @@ START_TEST(om) pNtQueryEvent = (void *)GetProcAddress(hntdll, "NtQueryEvent"); pNtPulseEvent = (void *)GetProcAddress(hntdll, "NtPulseEvent"); pNtOpenMutant = (void *)GetProcAddress(hntdll, "NtOpenMutant");
- pNtQueryMutant = (void *)GetProcAddress(hntdll, "NtQueryMutant");
- pNtReleaseMutant = (void *)GetProcAddress(hntdll, "NtReleaseMutant"); pNtOpenFile = (void *)GetProcAddress(hntdll, "NtOpenFile"); pNtClose = (void *)GetProcAddress(hntdll, "NtClose"); pRtlInitUnicodeString = (void *)GetProcAddress(hntdll, "RtlInitUnicodeString");
@@ -1925,6 +2036,7 @@ START_TEST(om) test_query_object(); test_type_mismatch(); test_event();
- test_mutant(); test_keyed_events(); test_null_device();
} diff --git a/server/mutex.c b/server/mutex.c index 3693095..b183934 100644 --- a/server/mutex.c +++ b/server/mutex.c @@ -251,3 +251,19 @@ DECL_HANDLER(release_mutex) release_object( mutex ); } }
+/* return details about the event */
Copy-paste mistake?
+DECL_HANDLER(query_mutex) +{
- struct mutex *mutex;
- if ((mutex = (struct mutex *)get_handle_obj( current->process, req->handle,
MUTANT_QUERY_STATE, &mutex_ops )))- {
reply->count = mutex->count;reply->owned = mutex->owner == current;
I usually prefer brackets around such comparisons, but not sure if its strictly required.
reply->abandoned = mutex->abandoned;release_object( mutex );- }
+} diff --git a/server/protocol.def b/server/protocol.def index a5a45eb..3359b77 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -1093,6 +1093,16 @@ enum event_op { PULSE_EVENT, SET_EVENT, RESET_EVENT }; @END
+/* Query a mutex */ +@REQ(query_mutex)
- obj_handle_t handle; /* handle to event */
+@REPLY
- unsigned int count; /* current count of mutex */
- int owned; /* true if owned by current thread */
- int abandoned; /* true if abandoned */
+@END
/* Create a semaphore */ @REQ(create_semaphore) unsigned int access; /* wanted access rights */
-
Sebastian Lackner