[PATCH 2/3] server: Add console process list ioctl.
Roman Pišl
rpisl at seznam.cz
Sat Feb 12 06:30:39 CST 2022
Hi Jacek,
thank you for the review. I've submitted a second version of the
patches, hopefully with your suggestions addressed.
Roman
Dne 08. 02. 22 v 22:32 Jacek Caban napsal(a):
> Hi Roman,
>
> On 2/4/22 19:03, Roman Pišl wrote:
>> Signed-off-by: Roman Pišl <rpisl at seznam.cz>
>> ---
>> include/wine/condrv.h | 1 +
>> server/console.c | 55 +++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 56 insertions(+)
>>
>> diff --git a/include/wine/condrv.h b/include/wine/condrv.h
>> index 4d2332a1ee9..13fb2c0b2c8 100644
>> --- a/include/wine/condrv.h
>> +++ b/include/wine/condrv.h
>> @@ -43,6 +43,7 @@
>> #define IOCTL_CONDRV_BEEP
>> CTL_CODE(FILE_DEVICE_CONSOLE, 20, METHOD_BUFFERED, FILE_ANY_ACCESS)
>> #define IOCTL_CONDRV_FLUSH
>> CTL_CODE(FILE_DEVICE_CONSOLE, 21, METHOD_BUFFERED, FILE_ANY_ACCESS)
>> #define IOCTL_CONDRV_GET_WINDOW
>> CTL_CODE(FILE_DEVICE_CONSOLE, 22, METHOD_BUFFERED, FILE_ANY_ACCESS)
>> +#define IOCTL_CONDRV_GET_PROCESS_LIST
>> CTL_CODE(FILE_DEVICE_CONSOLE, 23, METHOD_BUFFERED, FILE_ANY_ACCESS)
>> /* console output ioctls */
>> #define IOCTL_CONDRV_WRITE_CONSOLE
>> CTL_CODE(FILE_DEVICE_CONSOLE, 30, METHOD_BUFFERED, FILE_WRITE_ACCESS)
>> diff --git a/server/console.c b/server/console.c
>> index 5407fba1411..2df8fed5dfb 100644
>> --- a/server/console.c
>> +++ b/server/console.c
>> @@ -707,6 +707,27 @@ static void propagate_console_signal( struct
>> console *console,
>> enum_processes(propagate_console_signal_cb, &csi);
>> }
>> +struct console_process_list
>> +{
>> + DWORD size;
>> + DWORD count;
>> + DWORD *processes;
>> + struct console *console;
>> +};
>
>
> Please avoid using DWORD for internal server fields (size and count).
>
>
>> +
>> +static int console_process_list_cb(struct process *process, void *user)
>> +{
>> + struct console_process_list* cpl = (struct
>> console_process_list*)user;
>
>
> The cast is not needed.
>
>
>> +
>> + if (process->console == cpl->console)
>> + {
>> + if (cpl->count < cpl->size) cpl->processes[cpl->count] =
>> process->id;
>> + cpl->count++;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> /* dumb dump */
>> static void console_dump( struct object *obj, int verbose )
>> {
>> @@ -963,6 +984,40 @@ static void console_ioctl( struct fd *fd,
>> ioctl_code_t code, struct async *async
>> return;
>> }
>> + case IOCTL_CONDRV_GET_PROCESS_LIST:
>> + {
>> + struct console_process_list cpl;
>> + cpl.count = 0;
>> + cpl.size = get_reply_max_size() / sizeof(DWORD);
>> +
>> + if (cpl.size < 1)
>> + {
>> + set_error( STATUS_INVALID_PARAMETER );
>> + return;
>> + }
>> + if (!console)
>> + {
>> + set_error( STATUS_INVALID_HANDLE );
>> + return;
>> + }
>
>
> There is no need to test for NULL console here.
>
>
>> +
>> + cpl.console = console;
>> + cpl.processes = (DWORD *) set_reply_data_size(
>> get_reply_max_size() );
>> + enum_processes( console_process_list_cb, &cpl );
>> +
>> + if (cpl.count > cpl.size)
>> + {
>> + current->reply_size = sizeof(DWORD);
>> + *cpl.processes = cpl.count;
>> + set_error( STATUS_BUFFER_TOO_SMALL );
>> + return;
>> + }
>> +
>> + current->reply_size = cpl.count * sizeof(DWORD);
>
>
> over-allocating result and direct manipulation of reply_size does not
> really look nice. The usual practice would be to test size first and
> allocate appropriate result when we know its size. In this case, it's
> probably easiest if you do another enum_processes() call first, just to
> calculate result size (another solution would be to keep track of
> attached processes in console object itself).
>
>
> Thanks,
>
> Jacek
>
More information about the wine-devel
mailing list