On Sat, Mar 21, 2020 at 5:18 PM Zebediah Figura z.figura12@gmail.com wrote:
On 3/21/20 11:06 AM, Vijay Kiran Kamuju wrote:
On Sat, Mar 21, 2020 at 4:49 PM Zebediah Figura z.figura12@gmail.com wrote:
Hello Vijay,
On 3/20/20 4:46 PM, Vijay Kiran Kamuju wrote:
From: Vijay Kiran Kamuju infyquest@gmail.com Subject: [PATCH v3 2/2] ntdll/tests: Add tests for ProcessQuotaLimits class Message-Id: CACfa+KKTLOLG6pTUJO-8b7ez43oSVRnbtPKJ9tALc3JqVcFodw@mail.gmail.com Date: Fri, 20 Mar 2020 22:46:50 +0100
Signed-off-by: Vijay Kiran Kamuju infyquest@gmail.com
From 6bada725c1bb59bb127729cd5f373c29868d880d Mon Sep 17 00:00:00 2001 From: Vijay Kiran Kamuju infyquest@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@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.
This is used for tracing the quota values on different test systems with various RAM configurations. If you recommend, I will remove this function.
I'm not necessarily suggesting to remove the traces, I just mean that you don't need to put them in a separate function.
Ok, I will do that as well.
+{
- 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.
I am testing these as per previous recommendations, but some of values change by RAM values.
I'm not sure I see the review you're referring to, but my point is that the last four members (MinimumWorkingSetSize, MaximumWorkingSetSize, PagefileLimit, TimeLimit) are compared against fixed values, so there's no point printing their actual values. If the values are different from what you expect, you'll already get a failure in the ok() messages below. If those values do differ across different machines, then you probably shouldn't be using ok() to check them.
Well, I have seen some presentations related to PagedPoolLimit, NonPagedPoolLimitMinimumWorkingSetSize, MaximumWorkingSetSize https://techcommunity.microsoft.com/t5/windows-blog-archive/pushing-the-limi... I forgot the link for the ppt presentation.
+}
+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, "alimits, sizeof(quotalimits), NULL);
- ok( status == STATUS_INVALID_HANDLE, "Expected STATUS_INVALID_HANDLE, got %08x\n", status);
- status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, 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.
I wanted to test on a new process rather than the current process.
Why?
I thought the winetest may take up lot of resources, which might change the limits for the current process. I will change it to use current process.
- memset("alimits, 0, sizeof(quotalimits));
- status = pNtQueryInformationProcess( process, ProcessQuotaLimits, "alimits, 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.
Will do the changes.
- 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.
Will do the changes.
- dump_quota_limits("Quota Limits","alimits);
I think we need less tracing in this test by default, not more. I'd recommend checking against winetest_debug > 1.
Will do the changes.
- memset("alimits, 0, sizeof(quotalimits));
- status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, 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.)
Will do the changes.
- ok( sizeof(quotalimits) == ReturnLength, "Inconsistent length %d\n", ReturnLength);
- memset("alimits, 0, sizeof(quotalimits));
- status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, 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("alimits, 0, sizeof(quotalimits));
- status = pNtQueryInformationProcess( GetCurrentProcess(), ProcessQuotaLimits, "alimits, 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