[prev in list] [next in list] [prev in thread] [next in thread]
List: wine-devel
Subject: Re: [PATCH 1/2] ntdll: Add NtQueryMutant
From: Sebastian Lackner <sebastian () fds-team ! de>
Date: 2016-04-29 12:55:30
Message-ID: 49740186-88e6-c9d2-d44e-43ed9fc96de6 () fds-team ! de
[Download RAW message or body]
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 */
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic