Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
v2: Recurse through child jobs and only add active processes. Assign proper value to NumberOfAssignedProcesses. Use memcpy for 64-bit to avoid type aliasing issues.
dlls/kernel32/tests/process.c | 9 --------- dlls/ntdll/unix/sync.c | 31 ++++++++++++++++++++++++++++--- server/process.c | 34 ++++++++++++++++++++++++++++++++++ server/protocol.def | 9 +++++++++ 4 files changed, 71 insertions(+), 12 deletions(-)
diff --git a/dlls/kernel32/tests/process.c b/dlls/kernel32/tests/process.c index 10786c5..83b9a9c 100644 --- a/dlls/kernel32/tests/process.c +++ b/dlls/kernel32/tests/process.c @@ -2698,15 +2698,11 @@ static void test_QueryInformationJobObject(void) pid_list->NumberOfProcessIdsInList = 42; ret = QueryInformationJobObject(job, JobObjectBasicProcessIdList, pid_list, FIELD_OFFSET(JOBOBJECT_BASIC_PROCESS_ID_LIST, ProcessIdList[1]), &ret_len); - todo_wine ok(!ret, "QueryInformationJobObject expected failure\n"); - todo_wine expect_eq_d(ERROR_MORE_DATA, GetLastError()); if (ret) { - todo_wine expect_eq_d(42, pid_list->NumberOfAssignedProcesses); - todo_wine expect_eq_d(42, pid_list->NumberOfProcessIdsInList); }
@@ -2721,17 +2717,12 @@ static void test_QueryInformationJobObject(void) { ULONG_PTR *list = pid_list->ProcessIdList;
- todo_wine ok(ret_len == FIELD_OFFSET(JOBOBJECT_BASIC_PROCESS_ID_LIST, ProcessIdList[2]), "QueryInformationJobObject returned ret_len=%u\n", ret_len);
- todo_wine expect_eq_d(2, pid_list->NumberOfAssignedProcesses); - todo_wine expect_eq_d(2, pid_list->NumberOfProcessIdsInList); - todo_wine expect_eq_d(pi[0].dwProcessId, list[0]); - todo_wine expect_eq_d(pi[1].dwProcessId, list[1]); } } diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 16635ee..4592e8b 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -781,11 +781,36 @@ NTSTATUS WINAPI NtQueryInformationJobObject( HANDLE handle, JOBOBJECTINFOCLASS c case JobObjectBasicProcessIdList: { JOBOBJECT_BASIC_PROCESS_ID_LIST *process = info; + DWORD count, i;
if (len < sizeof(*process)) return STATUS_INFO_LENGTH_MISMATCH; - memset( process, 0, sizeof(*process) ); - if (ret_len) *ret_len = sizeof(*process); - return STATUS_SUCCESS; + + count = len - FIELD_OFFSET(JOBOBJECT_BASIC_PROCESS_ID_LIST, ProcessIdList); + count /= sizeof(process->ProcessIdList[0]); + + SERVER_START_REQ( query_job_pids ) + { + req->job = wine_server_user_handle(handle); + wine_server_set_reply(req, process->ProcessIdList, count * sizeof(process_id_t)); + if ((ret = wine_server_call(req)) != STATUS_SUCCESS) + return ret; + process->NumberOfAssignedProcesses = reply->count; + process->NumberOfProcessIdsInList = min(count, reply->count); + } + SERVER_END_REQ; + + if (sizeof(process_id_t) < sizeof(process->ProcessIdList[0])) + { + /* start from the end to not overwrite */ + for (i = process->NumberOfProcessIdsInList; i--;) + { + ULONG_PTR id = ((process_id_t*)(&process->ProcessIdList[0]))[i]; + memcpy(&process->ProcessIdList[i], &id, sizeof(id)); + } + } + + if (ret_len) *ret_len = (char*)(&process->ProcessIdList[process->NumberOfProcessIdsInList]) - (char*)process; + return count < process->NumberOfAssignedProcesses ? STATUS_MORE_ENTRIES : STATUS_SUCCESS; } case JobObjectExtendedLimitInformation: { diff --git a/server/process.c b/server/process.c index 5b271b1..0baf662 100644 --- a/server/process.c +++ b/server/process.c @@ -286,6 +286,24 @@ static int process_in_job( struct job *job, struct process *process ) return process->job == job; }
+static data_size_t query_job_pids( struct job *job, data_size_t count, data_size_t maxcount, process_id_t *pids ) +{ + struct process *process; + struct job *j; + + LIST_FOR_EACH_ENTRY( process, &job->process_list, struct process, job_entry ) + { + if (process->end_time) continue; + if (count < maxcount) pids[count] = process->id; + count++; + } + + LIST_FOR_EACH_ENTRY( j, &job->child_job_list, struct job, parent_job_entry ) + count = query_job_pids( j, count, maxcount, pids ); + + return count; +} + static void add_job_process( struct job *job, struct process *process ) { struct job *j, *common_parent; @@ -1739,6 +1757,22 @@ DECL_HANDLER(process_in_job) release_object( process ); }
+/* get a list of the pids associated with the job */ +DECL_HANDLER(query_job_pids) +{ + struct job *job = get_job_obj( current->process, req->job, JOB_OBJECT_QUERY ); + process_id_t *pids; + data_size_t len; + + if (!job) return; + + reply->count = query_job_pids( job, 0, 0, NULL ); + + len = min( get_reply_max_size(), reply->count * sizeof(*pids) ); + if (len && ((pids = set_reply_data_size( len )))) + query_job_pids( job, 0, len / sizeof(*pids), pids ); +} + /* retrieve information about a job */ DECL_HANDLER(get_job_info) { diff --git a/server/protocol.def b/server/protocol.def index b5bc049..b73a2c2 100644 --- a/server/protocol.def +++ b/server/protocol.def @@ -3665,6 +3665,15 @@ struct handle_info @END
+/* Query a list of pids associated with the job */ +@REQ(query_job_pids) + obj_handle_t job; /* handle to the job */ +@REPLY + data_size_t count; /* number of processes associated with the job */ + VARARG(pids,uints); /* list of pids */ +@END + + /* Set limit flags on a job */ @REQ(set_job_limits) obj_handle_t handle; /* handle to the job */
On 6/22/21 20:00, Gabriel Ivăncescu wrote:
+static data_size_t query_job_pids( struct job *job, data_size_t count, data_size_t maxcount, process_id_t *pids ) +{
- struct process *process;
- struct job *j;
- LIST_FOR_EACH_ENTRY( process, &job->process_list, struct process, job_entry )
- {
if (process->end_time) continue;
if (count < maxcount) pids[count] = process->id;
count++;
- }
- LIST_FOR_EACH_ENTRY( j, &job->child_job_list, struct job, parent_job_entry )
count = query_job_pids( j, count, maxcount, pids );
- return count;
+}
The other job enumerations (for terminating jobs and sending completions) do it for children first on then for parent. Do you know that this should not be the case for job pids? I don't know if the pid sort order is important for anything or consistent on Windows, but if not testing the order explicitly I'd suggest to enumerate in the same order as in the other places.
Hi Paul,
On 22/06/2021 21:14, Paul Gofman wrote:
On 6/22/21 20:00, Gabriel Ivăncescu wrote:
+static data_size_t query_job_pids( struct job *job, data_size_t count, data_size_t maxcount, process_id_t *pids ) +{
- struct process *process;
- struct job *j;
- LIST_FOR_EACH_ENTRY( process, &job->process_list, struct process, job_entry )
- {
if (process->end_time) continue;
if (count < maxcount) pids[count] = process->id;
count++;
- }
- LIST_FOR_EACH_ENTRY( j, &job->child_job_list, struct job, parent_job_entry )
count = query_job_pids( j, count, maxcount, pids );
- return count;
+}
The other job enumerations (for terminating jobs and sending completions) do it for children first on then for parent. Do you know that this should not be the case for job pids? I don't know if the pid sort order is important for anything or consistent on Windows, but if not testing the order explicitly I'd suggest to enumerate in the same order as in the other places.
Thanks for the review, you're right, it was just an oversight. I'll resend v3.