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

[geoff]

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