[PATCH resend] ntdll: Implement JobObjectBasicProcessIdList for NtQueryInformationJobObject.

[Huw Davies]

On Thu, Aug 12, 2021 at 07:02:37PM +0300, Gabriel Ivăncescu wrote:
> diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c
> index a13e53a..0594a5e 100644
> --- a/dlls/ntdll/unix/sync.c
> +++ b/dlls/ntdll/unix/sync.c
> @@ -791,11 +791,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;

Returning out of a SERVER_*_REQ block looks odd.

> +            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];

                   ULONG_PTR id = ((process_id_t *)(process->ProcessIdList))[i];
looks simpler to understand to me.

> +                memcpy(&process->ProcessIdList[i], &id, sizeof(id));

                   process->ProcessIdList[i] = id;
should work just fine.

> +            }
> +        }
> +
> +        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 0870de5..48e632a 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( j, &job->child_job_list, struct job, parent_job_entry )
> +        count = query_job_pids( j, count, maxcount, pids );
> +
> +    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++;
> +    }
> +
> +    return count;
> +}
> +
>  static void add_job_process( struct job *job, struct process *process )
>  {
>      struct job *j, *common_parent;
> @@ -1741,6 +1759,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 );

Doesn't this just give you job->num_processes?

> +
> +    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 133d6ad..5d97cb5 100644
> --- a/server/protocol.def
> +++ b/server/protocol.def
> @@ -3671,6 +3671,15 @@ struct handle_info
>  @END
>  
>  
> +/* Query a list of pids associated with the job */
> + at REQ(query_job_pids)
> +    obj_handle_t job;             /* handle to the job */
> + at REPLY
> +    data_size_t count;            /* number of processes associated with the job */
> +    VARARG(pids,uints);           /* list of pids */
> + at END
> +

Rather than introduce a new request, couldn't you extend the
get_job_info request?

Huw.