[PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class

[Zebediah Figura]

Hello Vijay,

On 3/20/20 4:46 PM, Vijay Kiran Kamuju wrote:
> From: Vijay Kiran Kamuju <infyquest at gmail.com>
> Subject: [PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class
> Message-Id: <CACfa+KKTLOLG6pTUJO-8b7ez43oSVRnbtPKJ9tALc3JqVcFodw at mail.gmail.com>
> Date: Fri, 20 Mar 2020 22:46:50 +0100
> 
> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> 
> From 6bada725c1bb59bb127729cd5f373c29868d880d Mon Sep 17 00:00:00 2001
> From: Vijay Kiran Kamuju <infyquest at gmail.com>
> Date: Sat, 15 Feb 2020 10:03:51 +0100
> Subject: [PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class
> 
> Signed-off-by: Vijay Kiran Kamuju <infyquest at gmail.com>
> ---
>  dlls/ntdll/tests/info.c | 72 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)
> 
> diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c
> index 8dc8bad645..7d3278b32e 100644
> --- a/dlls/ntdll/tests/info.c
> +++ b/dlls/ntdll/tests/info.c
> @@ -1308,6 +1308,74 @@ static void test_query_process_basic(void)
>      ok( pbi.UniqueProcessId > 0, "Expected a ProcessID > 0, got 0\n");
>  }
>  
> +static void dump_quota_limits(const char *header, const QUOTA_LIMITS *pql)

Is there any particular reason this needs to be a separate function?
Note that unlike dump_vm_counters(), this is only called once.

> +{
> +    trace("%s:\n", header);
> +    trace("PagedPoolLimit            : %lu\n", pql->PagedPoolLimit);
> +    trace("NonPagedPoolLimit         : %lu\n", pql->NonPagedPoolLimit);
> +    trace("MinimumWorkingSetSize     : %lu\n", pql->MinimumWorkingSetSize);
> +    trace("MaximumWorkingSetSize     : %lu\n", pql->MaximumWorkingSetSize);
> +    trace("PagefileLimit             : %lu\n", pql->PagefileLimit);
> +    trace("TimeLimit                 : %s\n", wine_dbgstr_longlong(pql->TimeLimit.QuadPart));

You already test these last four members against fixed values below; I'm
not sure there's a point in printing their values in that case.

> +}
> +
> +static void test_query_process_quotalimits(void)
> +{
> +    NTSTATUS status;
> +    QUOTA_LIMITS quotalimits;
> +    ULONG ReturnLength;
> +    HANDLE process;
> +
> +    status = pNtQueryInformationProcess(NULL, ProcessQuotaLimits, NULL, sizeof(quotalimits), NULL);
> +    ok( status == STATUS_ACCESS_VIOLATION || status == STATUS_INVALID_HANDLE,
> +        "Expected STATUS_ACCESS_VIOLATION or STATUS_INVALID_HANDLE(W2K3), got %08x\n", status);
> +
> +    status = pNtQueryInformationProcess(NULL, ProcessQuotaLimits, &quotalimits, sizeof(quotalimits), NULL);
> +    ok( status == STATUS_INVALID_HANDLE, "Expected STATUS_INVALID_HANDLE, got %08x\n", status);
> +
> +    status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, &quotalimits, 2, &ReturnLength);
> +    ok( status == STATUS_INFO_LENGTH_MISMATCH, "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> +
> +    process = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, one_before_last_pid);
> +    if (!process)
> +    {
> +        trace("Could not open process with ID : %d, error : %u. Going to use current one.\n", one_before_last_pid, GetLastError());
> +        process = GetCurrentProcess();
> +        trace("ProcessQuotaLimits for current process\n");
> +    }
> +    else
> +        trace("ProcessQuotaLimits for process with ID : %d\n", one_before_last_pid);

I don't think there's a point in doing this for this test. See the
comment regarding one_before_last_pid at the top of the file.

> +
> +    memset(&quotalimits, 0, sizeof(quotalimits));
> +    status = pNtQueryInformationProcess( process, ProcessQuotaLimits, &quotalimits, sizeof(quotalimits), &ReturnLength);
> +    ok( status == STATUS_SUCCESS, "Expected STATUS_SUCCESS, got %08x\n", status);
> +    ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> +    ok(quotalimits.MinimumWorkingSetSize == 204800,"Got MinimumWorkingSetSize = %lu\n",quotalimits.MinimumWorkingSetSize);
> +    ok(quotalimits.MaximumWorkingSetSize == 1413120,"Got MaximumWorkingSetSize = %lu\n",quotalimits.MaximumWorkingSetSize);
> +    ok(quotalimits.PagefileLimit == -1,"Got PagefileLimit = %lu\n",quotalimits.PagefileLimit);

Instead of comparing unsigned values to -1, comparing them to ~0 or
SIZE_MAX might be better.

> +    ok(quotalimits.TimeLimit.QuadPart == -1,"Got TimeLimit = %s\n",wine_dbgstr_longlong(quotalimits.TimeLimit.QuadPart));
> +    CloseHandle(process);

Style nitpick, the spacing here and elsewhere is rather inconsistent.

> +    dump_quota_limits("Quota Limits",&quotalimits);

I think we need less tracing in this test by default, not more. I'd
recommend checking against winetest_debug > 1.

> +
> +    memset(&quotalimits, 0, sizeof(quotalimits));
> +    status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, &quotalimits, sizeof(quotalimits)*2, &ReturnLength);
> +    ok( status == STATUS_INFO_LENGTH_MISMATCH,
> +        "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
Style nitpick: it seems weird to break this line here when the previous
line is longer and unbroken; I'd break both lines or neither.

(Using shorter ok() messages might not be a bad idea, either.)

> +    ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> +
> +    memset(&quotalimits, 0, sizeof(quotalimits));
> +    status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, &quotalimits, sizeof(quotalimits)-1, &ReturnLength);
> +    ok( status == STATUS_INFO_LENGTH_MISMATCH,
> +        "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> +    ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> +
> +    memset(&quotalimits, 0, sizeof(quotalimits));
> +    status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, &quotalimits, sizeof(quotalimits)+1, &ReturnLength);
> +    ok( status == STATUS_INFO_LENGTH_MISMATCH,
> +        "Expected STATUS_INFO_LENGTH_MISMATCH, got %08x\n", status);
> +    ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
> +}
> +
>  static void dump_vm_counters(const char *header, const VM_COUNTERS *pvi)
>  {
>      trace("%s:\n", header);
> @@ -2590,6 +2658,10 @@ START_TEST(info)
>      trace("Starting test_query_process_basic()\n");
>      test_query_process_basic();
>  
> +    /* 0x1 ProcessQuotaLimits */
> +    trace("Starting test_query_process_quotalimits()\n");
> +    test_query_process_quotalimits();
> +
>      /* 0x2 ProcessIoCounters */
>      trace("Starting test_query_process_io()\n");
>      test_query_process_io();
> 
> -- 
> 2.25.2
>