[PATCH 1/2] ntdll: Add NtQueryMutant

Sebastian Lackner sebastian at fds-team.de
Fri Apr 29 07:55:30 CDT 2016


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 at 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 */
> + at REQ(query_mutex)
> +    obj_handle_t  handle;       /* handle to event */
> + at REPLY
> +    unsigned int count;         /* current count of mutex */
> +    int          owned;         /* true if owned by current thread */
> +    int          abandoned;     /* true if abandoned */
> + at END
> +
> +
>  /* Create a semaphore */
>  @REQ(create_semaphore)
>      unsigned int access;        /* wanted access rights */
> 




More information about the wine-devel mailing list