[PATCH 2/3] server: Added server-side raw input function implementations (try 4)

[Vincas Miliūnas]

On 06/20/2011 05:24 PM, Vitaliy Margolen wrote:
> On 06/20/2011 02:55 AM, Vincas Miliūnas wrote:
>> (see the message in the first part of the patch)
>> +struct raw_input
>> +{
>> +    struct list    entry;
>> +    unsigned short usage_page;
>> +    unsigned short usage;
>> +    BOOL           processed;
>> +    BOOL           retrieved;
>> +    RAWINPUT       raw;
>> +
>> +    /* common fields */
>> +    unsigned int   type;
>> +    unsigned int   message;
>> +    unsigned int   flags;
>> +    unsigned int   extra_info;
>> +
>> +    /* mouse fields */
>> +    unsigned int   point_x;
>> +    unsigned int   point_y;
>> +    unsigned int   mouse_data;
>> +    unsigned int   cursor_x;
>> +    unsigned int   cursor_y;
>> +
>> +    /* keyboard fields */
>> +    unsigned int   vk_code;
>> +    unsigned int   scan_code;
>> +};
> Mouse and keyboard fields should be a union to save space.
>
>> +#define NUM_RAW_DEVICES (sizeof( raw_devices ) / sizeof( struct raw_device ))
> Please use "sizeof(array)/sizeof(array[0])" construct instead.
>
>> +    UINT i, size_in_bytes;
> In server please use standard C types ("unsigned int" in this case).
>
>> +        /* Currently fake handles are provided, however they are only used for the GetRawInputDeviceInfo function,
>> +           thus it should not create any undesirable side effects */
>> +        result[i].hDevice = (HANDLE)raw_devices[i].handle;
> This doesn't sound right. What this handle can be used for?
>
>> +    if (!(raw = malloc( sizeof( *raw ) )))
> In server please use mem_alloc() instead.
>
>> +/* Unregister a raw input device */
>> +DECL_HANDLER(unregister_raw_input_device)
> I don't see handing of case where device is not found. Should you return 
> error instead?
>
>> +    LIST_FOR_EACH( registration_entry, &current->process->raw_registered )
>> +    {
>> +        struct raw_registration *registration = LIST_ENTRY( registration_entry, struct raw_registration, entry );
> You can use this instead:
> LIST_FOR_EACH_ENTRY( registration, &current->process->raw_registered, 
> struct raw_registration, entry )
>
>
>> +    /* Prevent unprocessed raw input entries from being queued indefinitely */
>> +    while (current->process->raw_inputs_len >= MAX_RAW_INPUT_QUEUE_LENGTH)
>> +    {
>> +        struct raw_input *head = LIST_ENTRY( list_head( &current->process->raw_inputs ), struct raw_input, entry );
>> +        list_remove( &head->entry );
>> +        current->process->raw_inputs_len -= 1;
>> +        free( head );
>> +    }
> Looks like you need a circular buffer (an array of fixed size with head and 
> tail pointers) instead of a list here.
>
> Vitaliy.
>
I've updated the patch and replied -
http://www.winehq.org/pipermail/wine-patches/2011-June/103346.html