* Gerd Hoffmann (kra...@redhat.com) wrote:
> > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> > > index f388a23c8e..de171a28dd 100644
> > > --- a/hw/input/ps2.c
> > > +++ b/hw/input/ps2.c
> > > @@ -1225,24 +1225,21 @@ static void ps2_common_reset(PS2State *s)
> > > static void ps2_common_post_load(PS2State *s)
> > > {
> > > PS2Queue *q = &s->queue;
> > > - int size;
> > > - int i;
> > > - int tmp_data[PS2_QUEUE_SIZE];
> > > + uint8_t i, size;
> > > + uint8_t tmp_data[PS2_QUEUE_SIZE];
> >
> > Hi Prasad,
> >
> > you don't need to change the invalid values to sane ones. Instead, make
> > ps2_common_post_load return an int (just like the .post_load member of
> > VMStateDescription). You can then detect out of range count/rptr/wptr
> > and return -1 for bad indices.
>
> Well, it's not that simple because older qemu versions had larger
> queues. So post_load accepts migrations with queues which are too big.
> It just clears the queue in that case, so the input events are dropped
> in that (highly unlikely) case.
>
> Also note that the current post_load is broken. tmp_data is int whereas
> q->data is uint8_t. So ...
>
> > > - memcpy(q->data, tmp_data, size);
>
> ... this memcpy doesn't work as intended. Nobody noticed in years.
Oops!
> v1 fixes this bug, v2 doesn't.
>
> So, I'm tempted to just rip out the whole backward compatibility buffer
> shuffling logic. Or take v1 of this patch.
I think v1 is OK.
Dave
> cheers,
> Gerd
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
