Re: [Qemu-devel] [PATCH 2/2] ps2: Fix mouse stream corruption due to lost data

[geoff@hostfission.com]

On 2018-05-07 22:34, Gerd Hoffmann wrote:
>> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
>> index 6edf046820..011290920f 100644
>> --- a/hw/input/ps2.c
>> +++ b/hw/input/ps2.c
>> @@ -192,12 +192,50 @@ void ps2_queue(PS2State *s, int b)
>>  {
>>      PS2Queue *q = &s->queue;
>> 
>> -    if (q->count >= PS2_QUEUE_SIZE - 1)
>> +    if (q->count == PS2_QUEUE_SIZE)
>> +    {
>> +        printf("Warning! PS2 Queue Overflow!\n");
>>          return;
>> +    }
> 
> Leftover debug printf?

Correct :), I will remove it.

> 
>> +void ps2_raise(PS2State *s)
>> +{
>> +    s->update_irq(s->update_arg, 1);
>> +}
>> +
>> +void ps2_queue_raise(PS2State *s, int b)
>> +{
>> +    ps2_queue(s, b);
>> +    s->update_irq(s->update_arg, 1);
>> +}
> 
> I'd suggest to keep the ps2_queue() name.  Makes the patch much smaller
> and easier to review.  Factor out the code to actually queue things to
> a new ps2_queue_noirq() function.
> 
>> +void ps2_queue_bytes(PS2State *s, const int length, ...)

Ack.

> 
> I'd prefer to not use vaargs here as gcc can't check the arguments 
> then.
> Suggest to just have ps2_queue_{2,3,4}() helpers instead to queue
> multibyte messages.

Ack.

> 
> cheers,
>   Gerd

Thanks,
   Geoff